Skip to content

NH-3931 - Invalid order of child inserts #582

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 3 commits into from
Mar 28, 2017

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Mar 26, 2017

Test cases and fix for NH-3931.

Test cases ported from HHH-9864, HHH-11216 and HHH-11585, relaxed in terms of batching expectations, and completed with some missing cases.

Fix not ported but elaborated from current NHibernate implementation.

This PR will hopefully replace the previous one, which was attempting a new sorting algorithm. This new algorithm was better at batching some cases, but was failing on many previously supported cases. And it was having an O(n^3) complexity over batched entities classes count, where current algorithm, even fixed, has an O(n) complexity over batched entities count.

@fredericDelaporte fredericDelaporte force-pushed the NH-3931-2 branch 2 times, most recently from 9937f7b to f7adbc4 Compare March 26, 2017 01:08
var propertyValues = action.State;
var propertyTypes = action.Persister.ClassMetadata.PropertyTypes;
var propertyTypes = action.Persister.EntityMetamodel?.PropertyTypes;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClassMetadata current NHibernate implementation is reaching the underlying EntityMetamodel.PropertyTypes. As ClassMetadata seems "more optional" than EntityMetamodel, better use EntityMetamodel directly. (CustomPersister on NHibernate side does not provide any of them, but on Hibernate side, it does provide EntityMetamodel while still not providing ClassMetadata.)

@fredericDelaporte fredericDelaporte force-pushed the NH-3931-2 branch 2 times, most recently from 200daa3 to 2d3d185 Compare March 26, 2017 11:18
@hazzik
Copy link
Member

hazzik commented Mar 30, 2017

@fredericDelaporte next time, if you want to preserve multiple commits on a PR, please make a merge commit.

So:

  1. If PR has a single commit - rebase on top of master (if it is already on top of master, just apply it without merge commit).
  2. If PR has multiple commits, and you want to preserve them - make a merge commit.
  3. If PR has multiple commits, but you do not want them, then squash PR to master.

@fredericDelaporte
Copy link
Member Author

Ok, understood.

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.

2 participants