LINQ: unable to cast object of type 'NHibernate.Action.DelayedPostInsertIdentifier' to type 'System.IConvertible

[fabiomaulo]

var alreadyStored = await session.Query<HistoricoMyEntity>().Where(h => h.User == user).ToListAsync();
The user, in this case, is an entity previously persisted via session.Persist in the same session.

The exception is :
Unable to cast object of type 'NHibernate.Action.DelayedPostInsertIdentifier' to type 'System.IConvertible'

Expected:
NH should recognize the entity and the DelayedPostInsertIdentifier, retrieve the ID and use it in the generated SQL.

[fredericDelaporte]

Does this happen with the default auto flush mode?

[fabiomaulo]

Flush on Commit.
IMO it is related to the change in the behavior of the Persist (v3.0.0)

[fredericDelaporte]

As you know, the semantic of flush mode Commit is to only flush on commit. So if you query on some entity requiring to be flushed for allowing generating the query, the correct behavior is to raise an error.

According to #1754, the correct semantic of Persist includes not causing any flush by itself, which a post insert identifier must cause to be retrieved. So either you have to flush the whole session manually, or ensure your persisted entities using a post insert identifier are flushed before querying on them, or switch the flush mode to Auto.

So, yes this is a consequence of the Persist change (correction indeed), which derives from the announced possible breaking change:

ISession.Persist and ISession.Merge will no more trigger immediate generation of identifier.

About:

NH should recognize the entity and the DelayedPostInsertIdentifier, retrieve the ID and use it in the generated SQL.

That is what should happen with FlushMode.Auto. But it should not happen with FlushMode.Commit, as it would violate that FlushMode semantic.

[fabiomaulo]

Hi Frederic.
If I understood correctly, in your opinion, it is normal that in front of a LINQ with entity equality, NH will throw an exception because it can't convert something he created for its needs.

The change about the semantic of Persist is not so easy to resolve btw I will investigate a little bit more about #1754 matter (perhaps since v5.3.0 the Persist becomes the SaveOrUpdate and the SaveOrUpdate becomes the persist).

[fabiomaulo]

Investigation done.
A new bug was introduced with #1754 because it does not take into account the transaction boundaries in the semantic of Persit (half correction indeed).
Probably few developers saw "the problem", for 15 years, because it is hard to see somebody working with a session without a BeginTransaction.
BTW I'm the guilty who have introduced the bug when somebody works outside a transaction.

[hazzik]

Let's discuss everything here. It is hard to follow conversation when it is spread in multiple places.

The behavior here explicitly falls into undefined behavior category according to documentation and specifications.

Just to get a grasp of ideas how we should define the behavior, I would like to get some questions answered. What is/should be the behavior of the following code if ID is not identity?

session.FlushMode = FlushMode.Commit;

using var tx = session.BeginTransaction();

var user = new User(); // ID of user is *not* identity
session.Persist(user);
var others = session.Query<Other>().Where(h => h.User == user).ToList();

tx.Commit();

Why the behavior could/should be different when the same code executed for identity ID? Why it could/should be different when the same code executed for "native" ID on different DBs?

---

A new bug was introduced with #1754 because it does not take into account the transaction boundaries in the semantic of Persit (half correction indeed).

As I've explained here the persist's behavior does conform to the specification and to that old forum topic.

The behavior of persist was fixed for outside of transaction case as it was a clear violation of the specification. And also the behavior was made consistent for different identity types.

The bug is likely here, in the linq provider and not in persist's behavior. But, again, this is also conforms to the specification:

3.6.2 Queries and FlushMode

The flush mode setting affects the result of a query as follows.
When queries are executed within a transaction, if FlushModeType.AUTO is set on the Query object, or if the flush mode setting for the persistence context is AUTO (the default) and a flush mode setting has not been specified for the Query object, the persistence provider is responsible for ensuring that all updates to the state of all entities in the persistence context which could potentially affect the result of the query are visible to the processing of the query. The persistence provider implementation may achieve this by flushing those entities to the database or by some other means. If FlushModeType.COMMIT is set, the effect of updates made to entities in the persistence context upon queries is unspecified.

...

If there is no transaction active, the persistence provider must not flush to the database.

So, the real question is: why do you want to use FlushMode.Commit, @fabiomaulo ?

---

But it should not happen with FlushMode.Commit, as it would violate that FlushMode semantic.

@fredericDelaporte apparently Commit is permitted doing flushes when inside a transaction. A quote from the same JSR-220 (3.2.3 Synchronization to the Database):

The persistence provider runtime is permitted to perform synchronization to the database at other times
as well when a transaction is active... If FlushModeType.COMMIT is specified, flushing will occur at transaction commit; the persistence provider is permitted, but not required, to perform to flush at other times. If there is no transaction active, the persistence provider must not flush to the database.

But I personally, would not want to allow this erosion of the behavior of FlushMode.Commit as it could lead to unintended consequences and possible breaking changes.

---

PS: could you at least share the NHibernate's stack trace of the exception?
PPS: welcome back.

[fabiomaulo]

@hazzik
I never leave the usage of NHibernate so, as a user, I'm not back. 😉 🤓
I still working with my old friend NHibernate even if I have embraced the wide-spread-persistence.

So, the real question is: why do you want to use FlushMode.Commit ?

As you can imagine, in my case, many classes (usecases, mediators, domain-events, services etc.) work inside a UnitOfWork/DbContext but no one of those classes knows it.
To avoid chaos, in the head of our developers, there are two firmed points:

1. transaction isolation level as Read committed
2. FlushMode.Commit

This case:

If you work with an ID that does not require going to DB (as plain GUID for example), the code sequence works correctly because there isn't a DelayedPostInsertIdentifier in place.
The problem here is the exception when the entity is not already persisted.
Probably a good solution is:

1. NH recognizes the DelayedPostInsertIdentifier and he knows that it refer to a no persisted entity
2. NH replaces the ID with a default(T) where T is the type of the ID (something hard to do if T represents a composite-id)

For sure NH should not throw an exception in front of a managed entity (entity attached to a session) used in a query as a parameter (no matter which is the FlushMode or the transaction usage).

[fredericDelaporte]

If I understood correctly, in your opinion, it is normal that in front of a LINQ with entity equality, NH will throw an exception because it can't convert something he created for its needs.

No, it is normal it throws because an invalid operation has been attempted by the application code. But I agree the currently thrown exception needs improvement.

[hazzik]

@fabiomaulo do you have a stack trace?

[fredericDelaporte]

I have added a test, see #3043.

[fredericDelaporte]

As you can imagine, in my case, many classes (usecases, mediators, domain-events, services etc.) work inside a UnitOfWork/DbContext but no one of those classes knows it.
To avoid chaos, in the head of our developers, there are two firmed points:

1. transaction isolation level as Read committed
2. FlushMode.Commit

By the way, I do not understand how it helps not introducing "chaos" in the head of developers, especially if the flush previously caused by Persist on delayed identity insert was considered a desirable feature even in Commit flush mode. In such case, depending on the id generator strategy, the entity would be flushed immediately or not. This causes quite a discrepancy in behavior, only because of a "minor" implementation detail: the entity id generator.

[fabiomaulo]

Dear Frederic, I can't change everything in a big prj I have not designed.
Perhaps you know how many times I have warned NH's users to not use auto-numeric POID.

BTW, the issue is there, and IMO the solution is not about pushing people to change POID or Flush-Mode in an existing prj working with NH since 2006.
My expectation is at the beginning of this issue, my proposal is here; obviously, NH's team can do what has been considered the correct solution for the benefit of it's users.