Skip to content
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

Unwanted implied commits may occur for Oracle and MySql #3474

Closed
Epicycle23 opened this issue Jan 30, 2024 · 10 comments · Fixed by #3485
Closed

Unwanted implied commits may occur for Oracle and MySql #3474

Epicycle23 opened this issue Jan 30, 2024 · 10 comments · Fixed by #3485

Comments

@Epicycle23
Copy link

Epicycle23 commented Jan 30, 2024

NHibernate Version: 5.4.2
.net framework 4.8

We observed that running ExecuteUpdate on a HQL-Query causes updates that have been done before to be committed afterwards.

session.BeginTransaction();
session.Merge(someEntity);
session.Flush();
session.CreateQuery("Update MultiTableEntity set property = value where id = 123").ExecuteUpdate();

Debugging into NHibernate I was able to track this down to ShouldIsolateTemporaryTableDDL at least it seems like it.
For some reason NHibernate tries to create a (temporary) table which apparently fails because it already exists but just logs the error as debug and continues.
Since creating tables in oracle is causing a commit this seems to be the issue.
I noticed that there is a function ShouldIsolateTemporaryTableDDL in the class AbstractStatementExecutor which apparently first checks the dialect for PerformTemporaryTableDDLInIsolation which returns false or rather null when using the oracle dialect which if I am understanding this correctly should return true because oracle has an implicit commit when doing DDL operations and therefore DDL should run in isolation.

protected virtual bool ShouldIsolateTemporaryTableDDL()
		{
			bool? dialectVote = Factory.Dialect.PerformTemporaryTableDDLInIsolation();
			if (dialectVote.HasValue)
			{
				return dialectVote.Value;
			}
			return Factory.Settings.IsDataDefinitionImplicitCommit;
		}
@Epicycle23
Copy link
Author

Epicycle23 commented Jan 31, 2024

After debugging more I found out that this only happens with the MultiTableUpdateExceutor because for some reason that I still don't understand only this class tries to create a temporary table every time a HQL query/update is executed contrary to the BasicExecutor.

@fredericDelaporte
Copy link
Member

The use of this executor may happen when inheritance is mapped, on types mapped as abstract or having subclasses mapped. Such cases may cause the data of the entity to be split in many tables.

Nonetheless, you may define a custom Oracle dialect deriving from the one you are currently using, to check if overriding the PerformTemporaryTableDDLInIsolation method to yield true solves the issue.

@Epicycle23
Copy link
Author

Epicycle23 commented Feb 3, 2024

So everyone using oracle needs a custom dialect when there is a build-in oracle dialect that doesn't reflect the behavior of a oracle database when it comes to PerformTemporaryTableDDLInIsolation ?
Is that what you are saying?
Already checked and confirmed it on our end that changing it to true will run the temporary table DDL in isolation and therefore does not commit anything else than the attempt to create a temporary table for internal use only.
What's the reason for not having PerformTemporaryTableDDLInIsolation return true in the build-in oracle dialect in the first place?
As far as I know oracle will always do an implicit commit when performing DDL especially when creating/deleting temporary tables with the exception of PRAGMA AUTONOMOUS_TRANSACTION being present.
I'm trying to understand why this is not considered a bug but rather expected behavior.

@fredericDelaporte
Copy link
Member

So everyone using oracle needs a custom dialect when there is a build-in oracle dialect that doesn't reflect the behavior of a oracle database when it comes to PerformTemporaryTableDDLInIsolation ?
Is that what you are saying?

No.

That may be a needed workaround for now, if that does solve anything. I was actually asking you to check this. So you have checked and confirm it solves your case, thanks.

What's the reason for not having PerformTemporaryTableDDLInIsolation return true in the build-in oracle dialect in the first place?

No idea, apart default dialect behavior. Things are that way since very long.

According to your writings, that would be a bug, especially if this Oracle commit behavior you explain on temp table DDL affects all of its versions, whatever the way they are configured.

But looking on Hibernate side, their OracleDialect does not override it either to handle it in isolation. (On their side, that is now getTemporaryTableDdlTransactionHandling that handles that.)

So, I find it strange that the trouble would have stayed for so long both on our side and on their side. Are you sure that is not a trouble specific to your Oracle server configuration or something else specific to your setup?

The best way to ascertain this would be to provide a test case triggering the failure (I mean, unwanted commit, see contributing), so we can check that does occur too with our CI.

@Epicycle23
Copy link
Author

Epicycle23 commented Feb 5, 2024

Sorry for the misunderstanding.
I am no Oracle expert, but I talked to our DBA and did a little research and both confirmed that Oracle does an implicit commit when creating/deleting temporary tables.
Other than PRAGMA AUTONOMOUS_TRANSACTION I am not aware of any configuration that would change that behavior.
We use NHibernate for about 10+ years now and only now discovered this because an exception occurred after the HQL update which we use very rarely and even more rarely with multi table entities.
In fact the entity with which we discovered this only recently changed to a multi table entity.
So in summary, yes I am pretty sure that this is not unique to our configuration/setup.
I may try to contribute but I can't make any promises since I have to get familiar with the process first which will take some time I usually don't have unfortunately.

PS: When looking at the comments of PerformTemporaryTableDDLInIsolation it states something about the JDBC driver usually handling this if I understand that correctly so maybe this is where the difference is/why it may not happen with java/Hibernate.
I also tried to set Factory.Settings.IsDataDefinitionImplicitCommit but couldn't find any way to set this.

@hazzik
Copy link
Member

hazzik commented Feb 5, 2024

@Epicycle23, at this stage we're just asking if to test on your setup if using a custom dialect with the setting enabled would solve your problem.

@Epicycle23
Copy link
Author

Yes it does solve the problem.

@hazzik
Copy link
Member

hazzik commented Feb 5, 2024

Thanks for confirming.

@fredericDelaporte
Copy link
Member

fredericDelaporte commented Feb 5, 2024

When looking at the comments of PerformTemporaryTableDDLInIsolation it states something about the JDBC driver usually handling this.

On NHibernate side, that is not the case. Hibernate code has been ported to .Net sometimes without properly cleaning comments irrelevant for .Net. (On Hibernate side there is some code determining this property from the database metadata, excepted the end result seems used nowhere...)

Factory.Settings.IsDataDefinitionImplicitCommit: this one could have been an alternative way of handling the trouble, through configuration, which is less optimal than having it handled by the dialect. But it is some dead code: nothing allows to configure it. (Unless using reflection.)

@fredericDelaporte fredericDelaporte added this to the next minor milestone Feb 5, 2024
fredericDelaporte added a commit to fredericDelaporte/nhibernate-core that referenced this issue Feb 6, 2024
fredericDelaporte added a commit to fredericDelaporte/nhibernate-core that referenced this issue Feb 6, 2024
@fredericDelaporte
Copy link
Member

Fixed by #3485.

@fredericDelaporte fredericDelaporte changed the title PerformTemporaryTableDDLInIsolation seems to be wrong for oracle dialect PerformTemporaryTableDDLInIsolation is wrong for Oracle dialect Feb 7, 2024
@fredericDelaporte fredericDelaporte changed the title PerformTemporaryTableDDLInIsolation is wrong for Oracle dialect Unwanted implied commits may occur for Oracle and MySql Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants