Skip to content

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

Merged
merged 2 commits into from
Nov 19, 2018

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented May 24, 2018

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, ...)

@fredericDelaporte fredericDelaporte force-pushed the NH-3150 branch 2 times, most recently from 9221858 to b9c88f3 Compare June 3, 2018 18:06
@fredericDelaporte

This comment has been minimized.

@hazzik hazzik self-requested a review June 13, 2018 12:37
@hazzik
Copy link
Member

hazzik commented Jun 13, 2018

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.

@fredericDelaporte
Copy link
Member Author

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.

@fredericDelaporte

This comment has been minimized.

@fredericDelaporte
Copy link
Member Author

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.)

@hazzik
Copy link
Member

hazzik commented Oct 30, 2018

The issue noted that this solution is a hack has not been resolved since I posted it.

@fredericDelaporte
Copy link
Member Author

The issue noted that this solution is a hack has not been resolved since I posted it.

In June:

Rebased, and fixup commits addressing review added.

I had reworked it right after your post. But now the fixups are squashed.

mickfold and others added 2 commits October 31, 2018 22:56
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>
@fredericDelaporte fredericDelaporte merged commit a56e683 into nhibernate:master Nov 19, 2018
@fredericDelaporte fredericDelaporte deleted the NH-3150 branch November 19, 2018 10:44
fredericDelaporte added a commit to fredericDelaporte/nhibernate-core that referenced this pull request Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NH-3150 - Select Post Insert Generator Improvements
3 participants