-
Notifications
You must be signed in to change notification settings - Fork 934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test case for NH-3972 - SQL error when selecting a column of a subclass when sibling classes have a column of the same name #2242
Conversation
@@ -94,11 +93,16 @@ public virtual string[] ToColumns(string propertyName) | |||
|
|||
protected void AddPropertyPath(string path, IType type, string[] columns, string[] formulaTemplates) | |||
{ | |||
typesByPropertyPath[path] = type; | |||
columnsByPropertyPath[path] = columns; | |||
if (!typesByPropertyPath.ContainsKey(path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be better if we copy from is Hibernate project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say yes.
According to https://hibernate.atlassian.net/browse/HHH-11544 (commit here) your fix doesn't cover all cases:
The reason for this, is that during the metamodel building hibernate currently resolves p.base to the first type it encounters i.e. either PolymorphicSub1 or PolymorphicSub2. This will make it work for one type, but fail for the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bahusoid Do we have interface MetadataImplementor
?
If not, I believe we need change a lot of file and maybe it's better if we open other PR, or for this case will be better we fixes on NH 6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have interface MetadataImplementor
I have no idea.
maybe it's better if we open other PR
Maybe I just don't understand your fix. Old logic - last path occurrence is used; your fix - first path occurrence is used. So doesn't it fix one case but breaks another (as I quoted in comment above)?
I mean will your fix work for all sub-classes:
var result = session.Query<DataRecord>().Select(x => new
{
x.Subject,
State1 = ((Incident)x).State.Description,
State2 = ((Change)x).State.Description,
}).ToArray();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So suggested fix is not valid so let's commit test case only.
Test case for #1189