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

Upsert only works for PostgreSQL #1768

Closed
DavideD opened this issue Sep 20, 2023 · 8 comments · Fixed by #1805
Closed

Upsert only works for PostgreSQL #1768

DavideD opened this issue Sep 20, 2023 · 8 comments · Fixed by #1805
Assignees
Labels
bug Something isn't working
Milestone

Comments

@DavideD
Copy link
Member

DavideD commented Sep 20, 2023

Related to PR #1765

We added an upsert method to the stateless session interfaces in Hibernate Reactive.
For example in Mutiny.StatelessSession#upsert.

The problem is that it seems to only work for PostgreSQL.
The test fails for the other databases because when it looks for the results in the db, it returns an empty list.

I've quickly checked the same test with Hibernate ORM and MySQL, and it seems to run different queries. Probably, not all databases support the operation. But we need to look into it.

@DavideD DavideD added the bug Something isn't working label Sep 20, 2023
@blafond blafond self-assigned this Sep 21, 2023
@blafond
Copy link
Member

blafond commented Sep 21, 2023

@DavideD upsert() works for PostgreSQL because reactiveMerge() is actually called and executes this SQL:
merge into Record as t using (select cast($1 as bigint) id, cast($2 as text) message) as s on (t.id=s.id) when not matched then insert (id, message) values (s.id, s.message) when matched then update set message=s.message

The PG DB test creates a MutationExecutor of type ReactiveMutationExecutorSingleBatched which kicks off the update/merge.

Other DB's create a MutationExecutor of type ReactiveMutationExecutorSingleSelfExecuting which ends up calling the default performReactiveNonBatchedOperations() that returns a voidFuture() which does not execute merge ...... commands and the select query's are empty.

I still don't know enough about this MutationExecutor framework to move toward a fix.

@DavideD
Copy link
Member Author

DavideD commented Sep 21, 2023

It seems that we need to replace the voidFuture() with a reactive version of AbstractMutationExecutero#performNonBatchedMutation

@DavideD
Copy link
Member Author

DavideD commented Sep 21, 2023

Ah no, sorry.... I confused the two methods

@DavideD
Copy link
Member Author

DavideD commented Sep 21, 2023

Are you sure that's the issue? Have you checked with Hibernate ORM and MySQL (for example)?

I'm asking because in Hibernate ORM the implementation of MutationExecutorSingleSelfExecuting#performNonBatchedOperations comes from AbstractMutationExecutor#performNonBatchedOperations that doesn't do anything.

Without running any code, it seems that returning voidFuture is correct.

@blafond
Copy link
Member

blafond commented Sep 21, 2023

I'll check back into ORM. thx

@blafond
Copy link
Member

blafond commented Oct 8, 2023

@DavideD
Pushed latest: -- see: https://github.com/blafond/hibernate-reactive/tree/upsert-mysql-1768

  • I was able to fix the performUpdate() and now the test runs for all DB's but ORACLE
  • I almost have the performDelete() method working, but not sure if I set up the test for it correctly. It's currently failing and I've commented out the test : testStageUpsertWithDelete(). The delete SQL is executed but there's an exception happening in the vertx client.

@DavideD
Copy link
Member Author

DavideD commented Oct 10, 2023

The test seems correct, but I think you've copied the parameter binding from the update. The logic in ORM is quite different though, there is a bindKeyValues in OptionalTableUpdateOperation that does something else.

Also, there are a couple of .thenAccept( v -> voidFuture() ) that should be .thenCompose(CompletionStages::voidFuture)

@blafond
Copy link
Member

blafond commented Oct 30, 2023

Pushed latest changes again to: blafond@b5ec092

  • Added another override in: ReactiveStandardDialectResolver to intercept/override the oracle translator with ReactiveOracleSqlAstTranslator. This allowed the logic to set the Table alias correctly and fix the SQL error

blafond added a commit to blafond/hibernate-reactive that referenced this issue Nov 1, 2023
blafond added a commit to blafond/hibernate-reactive that referenced this issue Nov 1, 2023
blafond added a commit to blafond/hibernate-reactive that referenced this issue Nov 2, 2023
blafond added a commit to blafond/hibernate-reactive that referenced this issue Nov 2, 2023
DavideD pushed a commit to DavideD/hibernate-reactive that referenced this issue Nov 28, 2023
DavideD added a commit to DavideD/hibernate-reactive that referenced this issue Nov 28, 2023
DavideD pushed a commit to DavideD/hibernate-reactive that referenced this issue Nov 28, 2023
DavideD pushed a commit to DavideD/hibernate-reactive that referenced this issue Nov 29, 2023
DavideD added a commit that referenced this issue Nov 29, 2023
@DavideD DavideD added this to the 2.2.1.Final milestone Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants