Introduce generateStatementsFromRows for extensions #2686
Conversation
fbiville
force-pushed
the
load-data-refactoring
branch
from
e05ee17
to
3151f7c
Compare
| } | ||
| } | ||
| if (rows.stream().anyMatch(LoadDataRowConfig::needsPreparedStatement)) { | ||
| // If we have only prepared statements and the database supports batching, let's roll |
There was a problem hiding this comment.
I think the refactoring exhibits a bit more clearly a potential bug that was already there (not sure it has practical implications at all).
"any prepared statements" gets conflated with "only prepared statements". As a result, regular statements will be skipped as soon as 1 prepared statement is created.
fbiville
force-pushed
the
load-data-refactoring
branch
from
3151f7c
to
f838d9b
Compare
|
Note: all existing tests for this class pass locally and I added an extra one. |
This will allow extensions to override just the statement generation while relying on the existing CSV parsing logic. This is needed by the Neo4j extension.
fbiville
force-pushed
the
load-data-refactoring
branch
from
f838d9b
to
9304140
Compare
…iquibase into fbiville-load-data-refactoring
There was a problem hiding this comment.
The changes look good. I moved the LoadDataRowConfig inner class definition to the bottom since that's where they tend to be in the liquibase code, and made databaseSupportsBatchUpdates() into just supportsBatchUpdates()
Otherwise, I see how you just moved around the existing code and appreciate the extra unit test.
|
@nvoxland do we need to update any docs here? |
|
No we don't need additional docs. This change will be captured in the generated javadocs well enough |
There was a problem hiding this comment.
- Change impacts only those who override generateStatementsFromRows from a Liquibase extension.
- New test added validating the override.
- Code refactored to be more readable (thanks, @fbiville !!).
- No further testing required.
APPROVED
This will allow extensions to override just the statement
generation while relying on the existing CSV parsing logic.
This is needed by the Neo4j extension.