Add Support to Upsert/Insert Ignore on PDB #386
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**Summary:** - Introduces the capability to perform batch insert operations with duplicated entries without throwing a unique constraint violation exception. This is achieved by adding a new prepared statement to a `MappedEntity` when it is loaded. This statement will upsert the duplicate entries and will be implemented to each engine accordingly. This way, this prepared statement (upsert) will be used in the batch processing associated with the entity when adding and flushing the entries, but using the upserting any duplicated entries. This commit introduces support for the H2 and PostgreSQL engines for this operation. - For the H2 engine, the prepared statement is the `MERGE` command which updates existing rows, and insert rows that don't exist. The documentation for this engine can be found [here](http://www.h2database.com/html/commands.html#merge_into) ```sql MERGE INTO <TARGET_ENTITY> (cols...) VALUES(vals...) ``` - For the PostgreSQL engine, the insert statement introduces a `ON CONFLICT` clause that specifies an alternative action to raising a unique violation or exclusion constraint violation error. For the implementation, it was added the `DO UPDATE` action which will updating the columsn to the new values. It uses the values of the special table `EXCLUDED` to update it to the new values. The documentation for this engine can be found [here](https://www.postgresql.org/docs/current/sql-insert.html) ```sql INSERT INTO <TARGET_ENTITY> (cols...) VALUES (vals...) ON CONFLICT DO UPDATE SET (cols...) = EXCLUDED.(cols...) ``` **Test:** - A new test was added to verify that a batch with duplicate entries successfully inserts only one entry and do not throw an exception. - The test suite was run for every supported engine (H2 and PostgreSQL).
- Reverts import order to avoid boilerplate diff.
- Minor changes in the H2Engine MERGE command. - Refactors the INSERT ON CONFLICT in teh PostgreSqlEngine class, so it updates instead of ignoring duplicate entries. - Refactors test.
- Fix method name.
- Fix tests: Adds `assumeTrue` so it ignores the engines for two tests where the upsert prepared statement is not implemented.
- Fix tests: Adds cockroach db to exception as it extends the Postgres implementation.
- Fix tests: Fixes the h2 assumption as it requires more than one h2 engine.
src/main/java/com/feedzai/commons/sql/abstraction/batch/AbstractBatch.java
Outdated
Show resolved
Hide resolved
src/main/java/com/feedzai/commons/sql/abstraction/engine/impl/H2Engine.java
Outdated
Show resolved
Hide resolved
src/main/java/com/feedzai/commons/sql/abstraction/engine/impl/H2Engine.java
Show resolved
Hide resolved
Comment on lines
344
to
346
| public void flushUpsert() { | ||
| logger.trace("Flush ignoring not available for MultithreadedBatch. Skipping ..."); | ||
| } |
There was a problem hiding this comment.
This cannot be that dead silent :)
If this is not implemented and if it is still being used, then something is not right -- and then, the callee logic should fail fast, so it can be patched as fast as possible. So, I would throw an UnsupportedOperationException.
(at very least, this needs to be logged and error)
src/main/java/com/feedzai/commons/sql/abstraction/engine/impl/PostgreSqlEngine.java
Outdated
Show resolved
Hide resolved
Comment on lines
+85
to
+99
| protected void processBatchUpsert(final DatabaseEngine de, final List<BatchEntry> batchEntries) throws DatabaseEngineException { | ||
| /* | ||
| Begin transaction before the addBatch calls, in order to force the retry of the connection if it was lost during | ||
| or since the last batch. Otherwise, the addBatch call that uses a prepared statement will fail. | ||
| */ | ||
| de.beginTransaction(); | ||
|
|
||
| for (final BatchEntry entry : batchEntries) { | ||
| de.addBatchUpsert(entry.getTableName(), entry.getEntityEntry()); | ||
| } | ||
|
|
||
| de.flushUpsert(); | ||
| de.commit(); | ||
| } | ||
|
|
There was a problem hiding this comment.
DRY - this is duplicated code and should be improved.
(if there is time before the release, I would not make it a priority, just letting a note)
There was a problem hiding this comment.
This requires some further analysis, right off the bat I can't change it as from what I saw it could take some refactoring.
There was a problem hiding this comment.
ack! - we can address this later.
- Addresses comments.
diogo-anjos-fdz
approved these changes
Jun 7, 2023
Comment on lines
+85
to
+99
| protected void processBatchUpsert(final DatabaseEngine de, final List<BatchEntry> batchEntries) throws DatabaseEngineException { | ||
| /* | ||
| Begin transaction before the addBatch calls, in order to force the retry of the connection if it was lost during | ||
| or since the last batch. Otherwise, the addBatch call that uses a prepared statement will fail. | ||
| */ | ||
| de.beginTransaction(); | ||
|
|
||
| for (final BatchEntry entry : batchEntries) { | ||
| de.addBatchUpsert(entry.getTableName(), entry.getEntityEntry()); | ||
| } | ||
|
|
||
| de.flushUpsert(); | ||
| de.commit(); | ||
| } | ||
|
|
There was a problem hiding this comment.
ack! - we can address this later.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add Support to Upsert/Insert Ignore on PDB
Summary:
Introduces the capability to perform batch insert operations with duplicated entries without throwing a unique constraint violation exception. This is achieved by adding a new prepared statement to a
MappedEntitywhen it is loaded. This statement will upsert the duplicate entries and will be implemented to each engine accordingly. This way, this prepared statement (upsert) will be used in the batch processing associated with the entity when adding and flushing the entries, but using the upserting any duplicated entries. This commit introduces support for the H2 and PostgreSQL engines for this operation.For the H2 engine, the prepared statement is the
MERGEcommand which updates existing rows, and insert rows that don't exist. The documentation for this engine can be found hereON CONFLICTclause that specifies an alternative action to raising a unique violation or exclusion constraint violation error. For the implementation, it was added theDO UPDATEaction which will updating the columsn to the new values. It uses the values of the special tableEXCLUDEDto update it to the new values. The documentation for this engine can be found hereTest: