Fix for DAT-15716 :: Include 'OR REPLACE' instruction for a view when generate-changelog/diff-changelog command are executed #5304
Conversation
…sing generate-changelog or diff-changelog commands.
| If true, will add 'OR REPLACE' option to the given | ||
| stored logic change object for example, create | ||
| view, create procedure, etc.) |
There was a problem hiding this comment.
Should we include something in the description that says how it will play with the --always-drop-instead-of-replace argument?
There was a problem hiding this comment.
Good point, I haven't tested this change alongside that other existent configuration. I'll do that and see how to provide a better description for this new config.
Thanks,
Daniel.
There was a problem hiding this comment.
I haven't updated this one yet, I'm trying to check this one with Mario. I think there is no relationship between these two configurations If I'm not wrong ALWAYS_DROP_INSTEAD_OF_REPLACE is mostly used for update command (for example, when changing the structure of a view and we don't want to get an error in the try of updating a new version of it). Whereas the new USE_OR_REPLACE_OPTION configuration is used for generate-changelog and diff-changelog commands.
There was a problem hiding this comment.
We should probably explain that in the description, at least in the description of the property that you're adding.
There was a problem hiding this comment.
Well, I finally removed that global configuration and instead added it as a command argument of both (generate-changelog and diff-changelog) commands respectively. We should also reflect this in the docs.
liquibase-standard/src/main/java/liquibase/GlobalConfiguration.java
Outdated
Show resolved
Hide resolved
…gument of GenerateChangelog and DiffChangelog commands. - Tests updated.
liquibase-standard/src/main/java/liquibase/sqlgenerator/core/CreateViewGenerator.java
Outdated
Show resolved
Hide resolved
liquibase-standard/src/main/java/liquibase/diff/output/changelog/DiffToChangeLog.java
Outdated
Show resolved
Hide resolved
- Close DB connection on DiffChangelog integration tests.
- Command argument description updated.
# Conflicts: # liquibase-integration-tests/src/test/groovy/liquibase/diffchangelog/DiffChangelogIntegrationTest.groovy
…changelog and diff-changelog command.
|
Guys, as discussed in DAT-15716 I have added support in this PR for using Thanks, |
| USE_OR_REPLACE_OPTION = builder.argument("useOrReplaceOption", Boolean.class) | ||
| .description("If true, will add 'OR REPLACE' option to the create view change object") | ||
| .defaultValue(false) | ||
| .setValueHandler(ValueHandlerUtil::booleanValueHandler) | ||
| .build(); |
There was a problem hiding this comment.
It isn't necessary for this PR, but instead of repeating the argument, you can make a new class that extends AbstractArgumentCommandStep, and then specify that as a requiresDependency here and in every other class which requires this argument. This helps to reduce the code duplication.
There was a problem hiding this comment.
I created an abstract class with some common behavior that GenerateChangelogCommandStep and DiffChangelogCommandStep both use, I was thinking once that PR is merged to move this new argument there, and then in these two command classes do this:
builder.addArgument(AbstractChangelogCommandStep. USE_OR_REPLACE_OPTION).build();
What do you think about it?
Thanks,
Daniel.
There was a problem hiding this comment.
That sounds good. I don't think that's been done before, but it sounds simpler.
There was a problem hiding this comment.
It is! @filipelautert showed me yesterday that was already applied to RollbackCommandStep, so I applied the same to GenerateChangelog and DiffChangelog command step classes.
USE_OR_REPLACE_OPTIONcommand argument added to control whether we want to includeOR REPLACESQL instruction for a VIEW when executing agenerate-changelogordiff-changelogcommand.Integration tests were added for both commands.
Impact
Description
Things to be aware of
Things to worry about
Additional Context