Skip to content

Conversation

andrew-james-dev
Copy link
Contributor

@andrew-james-dev andrew-james-dev commented Oct 21, 2021

This PR adds missing supports implementations within the Spanner SQL generator. Without these methods, Liquibase will always generate code in the Spanner dialect without taking into account the type of database.

Whilst this is not an issue when Spanner is the only configured database, when there are multiple database types in use these missing methods cause Liquibase to incorrectly use the Spanner dialect, breaking migration functionality.

@google-cla google-cla bot added the cla: yes CLA signed label Oct 21, 2021
@skuruppu skuruppu requested a review from olavloite October 21, 2021 02:40
Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. LGTM (with a super-minor nit on an additional empty line)

}


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove extraneous empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be resolved now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@olavloite olavloite merged commit ad1facd into cloudspannerecosystem:master Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants