NH-2534 - Join-fetching a many-to-one with property-ref results in se… #1460

kfedorchenko · 2017-11-29T11:21:37Z

…lect n+1 problem (#1226)

…lect n+1 problem (nhibernate#1226) fixed nhibernate#1226

fredericDelaporte · 2017-11-29T11:32:50Z

src/NHibernate.Test/NHibernate.Test.csproj

  </PropertyGroup>
  <ItemGroup>
+    <Compile Remove="NHSpecificTest\GH01226\**" />
+    <EmbeddedResource Remove="NHSpecificTest\GH01226\**" />


Hi,
I do not think the changes on the test csproj are needed. Please revert them or explain them.

Yes, it is wrong lines. I will fix them

…lect n+1 problem (nhibernate#1226) fix NHibernate.Test.csproj

fredericDelaporte · 2017-11-29T12:05:29Z

src/NHibernate.Test/NHibernate.Test.csproj

  </ItemGroup>
+  <ItemGroup>
+    <Folder Include="NHSpecificTest\GH1226\" />
+  </ItemGroup>


Thanks, but, this change too should not be needed. We have migrated to the new csproj format also for ceasing having changes in it just for adding a test. Is there a specific reason this test would require this change while the other tests do not?

I missed this changes in the VS file format, thanks

…lect n+1 problem (nhibernate#1226) fix NHibernate.Test.csproj

See nhibernate#1459

…is failed (nhibernate#1462)

Fixes nhibernate#1468

* Fix ProxyFactory cache - use a correct implementation of cache - reduce the number of allocations of ProxyCacheEntry - reduce memory flow

* Always pass format arguments to the logger. * Enable better structured logging. * Update NHibernate.Example.Web to use new INHibernateLoggerFactory.

* Fixes nhibernate#1335

- Make methods of UpdateTimestampsCache virtual - Mark some methods UpdateTimestampsCache obsolete

…bernate#1477)

fredericDelaporte · 2017-12-11T12:08:53Z

I have re-formatted the test case for them to comply with contributing guideline. Their async counterparts are lacking, but I cannot regenerate the async files because your PR use an out-of-date base, 59 commits behind NHibernate master.

Please rebase your master for having it even with current NHibernate master, then rebase your PR (and squash its ~~four~~ eight commits together by the way, amend the result with an async regen as explained in contributing).

~~About changes in Loader, I would rather avoid the almost code duplication it represents. But I still have to better analyze this issue.~~

fredericDelaporte · 2017-12-11T13:13:31Z

src/NHibernate/Loader/Loader.cs

+					ILoadable entityPersister = (ILoadable) persister;
+					string[][] cols = EntityAliases[i].SuffixedPropertyAliases;
+
+					object[] values = entityPersister.Hydrate(rs, id, obj, entityPersister, cols, eagerPropertyFetch, session);


Here, there is a trouble. The referenced property may be mutable. If it has changed in DB but is still loaded in session with the old value, we are then going to associate a unique key on the new value with an entity still holding the old value. Instead of reading its value from the result set, it should be read from the entity in session (obj).

We need to test this case.

Done in #1492.

…cache.

fredericDelaporte · 2017-12-11T13:48:33Z

My last commit change the fix for a more proper implementation in my opinion.

kfedorchenko · 2017-12-11T13:55:47Z

@fredericDelaporte Thanks, I will make changes corresponding your recommendations. Also I did not make changes to corresponding async method InstanceAlreadyLoadedAsync.

fredericDelaporte · 2017-12-11T13:59:06Z

Also I did not make changes to corresponding async method InstanceAlreadyLoadedAsync.

Just in case, I remind you this is the job of the async generator, do not make them manually. Run the async generator instead. But this requires first to update this branch.

fredericDelaporte · 2017-12-11T17:19:46Z

My last commit causes a regression in a test, so it needs more work too.

fredericDelaporte · 2017-12-11T18:30:19Z

I have added the fix, everything is to be squashed in a single one (or will be squashed when merging).

- The factory generates high-efficient static proxies (no interceptor)

- Add tests for legacy logger - Don't split global state for logging. Fixes nhibernate#1478

Fixes nhibernate#1403

* Remove logic duplication for getting default NHibernate type for a CLR type. * Test all defaults for CLR type having many possibilities. * Clean-up.

…ibernate#1482)

* And fix ComponentType.IsModified(…) method throws exception when used with a component as "old" parameter. * Fix nhibernate#1486

fredericDelaporte · 2017-12-16T11:34:00Z

Please rebase your master for having it even with current NHibernate master, then rebase your PR (and squash its ~~four~~ eight commits together by the way, amend the result with an async regen as explained in contributing).

Done in #1492. If you take time doing it yourself on your PR, we will merge it. Otherwise my PR will likely be merged and this one closed as replaced.

…lect n+1 problem (nhibernate#1226) fixed nhibernate#1226

fredericDelaporte · 2017-12-18T11:19:36Z

Hi,

Commits from master should not appear here. I guess something went wrong in the way this branch was brought up-to-date with master.

It should look like in #1492, where only the changes of this PR appear, while still having the latest commit from master.

May you try re-basing again?

Assuming your local branches are named the same than your remotes one, this should be doable with:

git checkout GH1226
git rebase master

Here you may have a bunch of conflict to solve. Solve each conflict, re-add each file which were in conflict (stage it), then continue with:

git rebase --continue

This may lead to another conflicts to resolve, in such case resolve those other ones the same way.
The conflict resolution may also cause the conflicting commit to went empty, which would then prevent continuation: in such case the commit should be dropped with:

git rebase --skip

And ultimately the rebase will end.
At that point, you should then update your remote with a forced push. A normal push will be rejected, telling you to pull before, but do not do that. When rebasing a branch having its own commits, the remote must be overridden with a forced push.

git push -f

hazzik · 2017-12-22T02:42:13Z

Duplicate of #1492

NH-2534 - Join-fetching a many-to-one with property-ref results in se…

00d78cb

…lect n+1 problem (nhibernate#1226) fixed nhibernate#1226

fredericDelaporte reviewed Nov 29, 2017

View reviewed changes

NH-2534 - Join-fetching a many-to-one with property-ref results in se…

b967fc0

…lect n+1 problem (nhibernate#1226) fix NHibernate.Test.csproj

fredericDelaporte reviewed Nov 29, 2017

View reviewed changes

kfedorchenko and others added 14 commits November 29, 2017 14:20

NH-2534 - Join-fetching a many-to-one with property-ref results in se…

8c0d711

…lect n+1 problem (nhibernate#1226) fix NHibernate.Test.csproj

Fix merging of 5.0.2

ef86fdf

See nhibernate#1459

Fix disposing SessionIdLoggingContext if CheckAndUpdateSessionStatus …

63dee05

…is failed (nhibernate#1462)

Release 5.0.3

942b61b

Merge tag '5.0.3'

4edbbed

Fix a null reference case in session context

338a827

Obsolete an unused method.

7af6f4c

Obsolete EqualsHelper

dfe5dec

Obsolete EnumerableExtensions (nhibernate#1465)

524d06a

Fix generating coalesce for Boolean expressions

196a1c4

Fixes nhibernate#1468

Fix ProxyFactory cache (nhibernate#1454)

970e9fa

* Fix ProxyFactory cache - use a correct implementation of cache - reduce the number of allocations of ProxyCacheEntry - reduce memory flow

Logging refactoring (nhibernate#1377)

a058d38

* Always pass format arguments to the logger. * Enable better structured logging. * Update NHibernate.Example.Web to use new INHibernateLoggerFactory.

Upgrade AsyncGenerator to 0.7.0

433921a

NH-3787 - Decimal truncation in Linq ternary expression (nhibernate#707)

293c2eb

* Fixes nhibernate#1335

fredericDelaporte self-requested a review December 7, 2017 21:10

fredericDelaporte and others added 4 commits December 8, 2017 00:02

Document expiration constraint on UpdateTimestampsCache region.

bf7e34d

Reduce the number of calls to UpdateTimestampsCache (nhibernate#1467)

73797df

- Make methods of UpdateTimestampsCache virtual - Mark some methods UpdateTimestampsCache obsolete

Reuse the same generic EmptyMapClass instance across the project (nhi…

c4eb6f4

…bernate#1477)

Reformated test case, to be squashed.

f6db500

Removed unneeded id assignation.

bcad8ba

fredericDelaporte reviewed Dec 11, 2017

View reviewed changes

Cache the unique key from entity property when the entity comes from …

f61fa8b

…cache.

fredericDelaporte self-assigned this Dec 11, 2017

fredericDelaporte added the t: Fix label Dec 11, 2017

fredericDelaporte mentioned this pull request Dec 11, 2017

Ability to select entities in Criteria projections #1471

Merged

Support entity not yet fully loaded and un-associated.

3921f6c

hazzik and others added 7 commits December 13, 2017 13:04

Implement new StaticProxyFactoryFactory (nhibernate#1451)

bc30eb4

- The factory generates high-efficient static proxies (no interceptor)

Fix regression caused by logging refactoring (nhibernate#1479)

906c692

- Add tests for legacy logger - Don't split global state for logging. Fixes nhibernate#1478

Add timeouts support to MultiCriteria

c48b4fd

Fixes nhibernate#1403

Fix default types (nhibernate#1484)

6d8d5d9

* Remove logic duplication for getting default NHibernate type for a CLR type. * Test all defaults for CLR type having many possibilities. * Clean-up.

Refactored DefaultEntityAliases to avoid unnecessary calculations (nh…

aada800

…ibernate#1482)

Optimize empty arrays usages (nhibernate#1490)

418d35a

Fix IsModified so that null equates empty components

7e440bf

* And fix ComponentType.IsModified(…) method throws exception when used with a component as "old" parameter. * Fix nhibernate#1486

fredericDelaporte added the needs work label Dec 16, 2017

Forgotten async generation for nhibernate#1487

8ffa874

fredericDelaporte mentioned this pull request Dec 16, 2017

Join-fetching with property-ref results in select n+1 #1492

Merged

fredericDelaporte and others added 3 commits December 16, 2017 13:45

Adjustment for Join-fetching with property-ref results in n+1 select

92bae88

NH-2534 - Join-fetching a many-to-one with property-ref results in se…

a9e3698

…lect n+1 problem (nhibernate#1226) fixed nhibernate#1226

Merge remote-tracking branch 'origin/GH1226' into GH1226

946e054

hazzik marked this as a duplicate of #1492 Dec 22, 2017

hazzik closed this Dec 22, 2017

hazzik added the r: Replaced label Dec 22, 2017

NH-2534 - Join-fetching a many-to-one with property-ref results in se… #1460

NH-2534 - Join-fetching a many-to-one with property-ref results in se… #1460

Uh oh!

Conversation

kfedorchenko commented Nov 29, 2017

Uh oh!

fredericDelaporte Nov 29, 2017

Choose a reason for hiding this comment

Uh oh!

kfedorchenko Nov 29, 2017

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte Nov 29, 2017

Choose a reason for hiding this comment

Uh oh!

kfedorchenko Nov 29, 2017

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte commented Dec 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte Dec 11, 2017

Choose a reason for hiding this comment

Uh oh!

hazzik Dec 20, 2017

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte Dec 20, 2017

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte commented Dec 11, 2017

Uh oh!

kfedorchenko commented Dec 11, 2017

Uh oh!

fredericDelaporte commented Dec 11, 2017

Uh oh!

fredericDelaporte commented Dec 11, 2017

Uh oh!

fredericDelaporte commented Dec 11, 2017

Uh oh!

fredericDelaporte commented Dec 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte commented Dec 18, 2017

Uh oh!

hazzik commented Dec 22, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fredericDelaporte commented Dec 11, 2017 •

edited

Loading

fredericDelaporte commented Dec 16, 2017 •

edited

Loading