-
Notifications
You must be signed in to change notification settings - Fork 934
Fix NH-3150: select id generator improvements #1707
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
Fix NH-3150: select id generator improvements #1707
Conversation
9221858
to
b9c88f3
Compare
This comment has been minimized.
This comment has been minimized.
Overall solution looks like a hack (I'm not talking here about the workarounds to avoid breaking changes). It seems there could be a more elegant solution. |
That is a solution attempting minimal impact, likely because the author has not contributed much to NHibernate. I could have reworked it a bit more. |
b9c88f3
to
4fb8f57
Compare
This comment has been minimized.
This comment has been minimized.
458dc30
to
de222e2
Compare
Rebased again. Since it is also about unique key, we could fear some conflicts with changes done for associations with property-ref. But there are none, because here this is about natural-ids, not property-ref. As proposed on the development group, and since no continued grounded disagreement has been expressed there since one week after raised concerns being addressed, I intend to merge this PR next week (likely on Wednesday). (I am issuing this notice on PR issued before starting to apply this handling of merges. I will not do it for new PR.) |
The issue noted that this solution is a hack has not been resolved since I posted it. |
In June:
I had reworked it right after your post. But now the fixups are squashed. |
Co-Authored-by: Frédéric Delaporte <12201973+fredericdelaporte@users.noreply.github.com>
Co-Authored-by: Frédéric Delaporte <12201973+fredericdelaporte@users.noreply.github.com>
f19e293
to
4620c94
Compare
Fixes #822
Replaces #110
The select id generator generates the id by getting it from the database with a select query filtered by an unique key. This unique key could only be a single entity property, forcing to use a component if the key has to spawn many properties. The first commit add support for keys spanning many properties without using a component.
The select id generator was not supporting idbags, although some preliminary work was already done for this. The second commit fixes this.
These two commits are based on #110 commit, rebased then split and altered on many points. (Breaking changes removed, redundant casts removed, syntax modernized, multiple properties support added to the key parameter and not only to natural-id, tests adjusted, ...)