-
Notifications
You must be signed in to change notification settings - Fork 15
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
tgz: Implemented ColumnSnapshotGenerator to handle numeric boolean default values in snapshots #191
Conversation
a534232
to
d516533
Compare
d516533
to
b8acbb0
Compare
…fault values in snapshots
b8acbb0
to
f119596
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tgehrsitz-ite thanks for the PR ! I requested a small change to make sure it works as expected.
|
||
@Override | ||
public int getPriority(Class<? extends DatabaseObject> objectType, Database database) { | ||
return PRIORITY_DATABASE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to make sure that this extension priority doesn't break other databases the getPriority needs to check if the current database is hanadb, similar to this one in bigquey: https://github.com/liquibase/liquibase-bigquery/blob/dcc3eb960fd437115d1f22bc0f95f15dba943c6d/src/main/java/liquibase/ext/bigquery/snapshot/jvm/BigQueryViewSnapshotGenerator.java#L26
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @filipelautert, apologies for the delayed reply this time from my side. Good point, thanks for reviewing! I just pushed the changes to the PR/branch.
…_values_as_boolean_defaults
…g to HanaDatabase
Quality Gate passedIssues Measures |
PROPOSAL: when generating Snapshot, use true/false for default boolean values instead of numeric ones.