Excessive SQL statements for component ID containing entities

[mattzink]

My colleague was able to create a standalone repro for #2173, but since that issue was closed, I'm opening a new one.

Our software is seeing an inflated number of SQL statements being issued, in very particular situations, after upgrading from NHibernate 5.1.6 to 5.2.0+.

.NET Core 3.0 + SQLite repro source: nh-repro.zip

Program.Main() contains 3 NH operations that together demonstrate the difference:

long startingStatementCount = sessionFactory.Statistics.PrepareStatementCount;

using (var session = sessionFactory.OpenSession())
using (session.BeginTransaction())
{
    var parent = session.QueryOver<Parent>().List()[0];

    session.Refresh(parent);

    session.QueryOver<Parent>().Future().ToList();
}

Console.WriteLine($"SQL statements issued: {sessionFactory.Statistics.PrepareStatementCount - startingStatementCount}");

Executing the code using NH 5.1.6 shows the expected number of executed statements:

$ dotnet run -p nh-5.1.6
SQL statements issued: 3

NH 5.2.0 is the first version to show additional statements:

$ dotnet run -p nh-5.2.0
SQL statements issued: 7

NH 5.2.6 still exhibits the defect:

$ dotnet run -p nh-5.2.6
SQL statements issued: 7

Some conditions that appear to be required to reproduce the issue:

1. The queries/refresh must be in a transaction
2. The 2nd query must use .Future()
3. A related entity with a composite key, whose mapping from the first entity cascades both Refresh (or All) and DeleteOrphans
4. Another related entity from that one that overrides Equals and GetHashCode

[bahusoid]

2. The 2nd query must use .Future()

I've replaced
session.QueryOver<Parent>().Future().ToList(); with session.QueryOver<Parent>().ToList(); and see no difference in behavior. Future has nothing to do with this behavior.

[bahusoid]

Also I've run your test against master - I see no excessive statements. So seems it's something that's been fixed since 5.2 release.

Pretty busy this month so that's all investigitions from me...

[strawd]

@bahusoid the difference when not using Future() is that NHibernate 5.1.6 and other prior versions also show the excessive statements in that case. But since our application code uses a Future we saw a difference between 5.1 and 5.2.

That's great to hear that the issue is not reproducible on master.

[mattzink]

Thanks @bahusoid for your help.

I have also verified this is fixed with latest CI build. I did a quick binary search of CI builds to find the first one where the issue doesn't appear. It was Build 2598138, which means either PR #2246 or PR #2056 was what fixed this issue.

[bahusoid]

Interesting... #2246 is surely unrelated. And #2056 was not about fixing any issues... So there is a chance that issue is still present in master but it's hidden by optimization done in #2056. So it would be useful if you can bisect CI build when this issue appeared in 5.2.

Update Actually #2056 fixed those excessive queries by using proper GetHashCode override taking sessionFactory as parameter.

[bahusoid]

the difference when not using Future() is that NHibernate 5.1.6 and other prior versions also show the excessive statements in that case.

Ah... That's actually most important part. Before 5.2 auto flushing logic was ignored in Future queries for Criteria and this behavior is fixed in 5.2 (issue #865) - that's why you see additional queries in 5.2. So you can avoid additional queries in 5.2 by specifying FlushMode.Manual (via setting default_flush_mode or per each session creation).

Just adding this for future reference: about excessive SQL statements - they generated on identifierType.GetHashCode(obj) access.
Where identifierType is ComponentType with not loaded entity of ManyToOneType inside:

//Child entity mapping
ComposedId(id =>
{
    id.ManyToOne(x => x.Parent, map =>
    {
        map.Insert(false);
        map.Update(false);
    });

    // This Foo entity is not initialized on GetHashCode call
    id.ManyToOne(x => x.Foo, map =>
    {
        map.Insert(false);
        map.Update(false);
    });
});

So identifierType.GetHashCode(obj) leads to Foo.GetHashCode() and that leads to proxy initialization.
In 5.3 it's fixed for GetOrphans by calling identifierType.GetHashCode(obj, sessionFactory) (with sessionFactory param) which handles entities without initialization. Need to make sure we always call override with sessionFactory.

[bahusoid]

Closing it because excessive statements issue is very old - it's not a regression so #2056 won't be merged to 5.2. And the fact you didn't see it with Future is because of another issue #865 that was fixed in 5.2 (to have 5.1 behavior for Future criteria queries set flush mode to FlushMode.Manual)

So possible excessive SQL statements inside flush are fixed with #2056 in 5.3

[mattzink]

So is the expectation that users of NH 5.2 who don't want 2-11x more SQL statements sent to their DB set FlushMode.Manual on every session?

Is there a rough ETA on a 5.3 release where this workaround won't be required?

[mattzink]

Also, @bahusoid, would you mind explaining how FlushMode is involved in transactions that only contain queries (like in our repro)? I'm afraid I don't think I understand NH well enough.

Nevermind. I read the code and figured out my misunderstanding.

[bahusoid]

So is the expectation that users of NH 5.2 who don't want 2-11x more SQL statements sent to their DB set FlushMode.Manual on every session?

The fact that List and criteria Future behavior is different in versions prior to 5.2 is a bug. So users that were using this bug intentionally or not - yes should adjust their code in 5.2.

With FlushMode.Auto some checks are executed before query execution to check if session is dirty and Flush is required. For query only sessions it's better to avoid those checks altogether with or without fix made in 5.3.

P.S. All is IMHO

[mattzink]

Thank you for your further explanation, @bahusoid. It looks like we should be able to use FlushMode.Commit safely in almost all of our cases (to support mixed query/update transactions) and still avoid the extra SQL calls.

For others who hit this same issue, you can programmatically override the default FlushMode for all sessions during ISessionFactory configuration with:

configuration.SetProperty(NHibernate.Cfg.Environment.DefaultFlushMode, FlushMode.Commit.ToString());