Skip to content
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

SQL Anywhere: Switching off view definition checks to allow creation of dependent views even if dependency does not yet exist. #4537

Merged
merged 5 commits into from Aug 29, 2023

Conversation

mkarg
Copy link
Contributor

@mkarg mkarg commented Jul 25, 2023

Fixes #4536

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This PR introduces the exact same solution as SQL Anywhere's native schema-creation tool dbunload is using in the same situation: It temporarily switches off the check whether the dependencies found in the view definition already do exist.

See https://help.sap.com/docs/SAP_SQL_Anywhere/61ecb3d4d8be4baaa07cc4db0ddb5d0a/813bf68b6ce21014b288ca2a8ab530f9.html. While it says, this option is just for the use within reload.sql and dbunload, the fact that it is documented proofs that SAP is pretty aware of the fact that there is no other solution to the problem, and the solution must be usable outside of the reload.sql file. In fact, reload.sql in a nutshell could be seen as the output of update-sql, so our use is rather valid.

Things to be aware of

SQL Anywhere will not complain anymore about any missing dependencies when creating views. Maybe someone wants update to fail? OTOH if he wants such a check, such a check would be a new feature for validate. IMHO it is a misuse of update to rely on its existing bugs, and the original SAP tool does neither provides such a checking feature!

Things to worry about

Paraphrasing SAP's documentation, this should not get used uncoditionally with all CREATE VIEW, but only with the ones created by generateChangelog. So a 1:1 copy of SAP's native solution would not be to enable that option in update, but to create a <sql dbms='asany'> change by generateChangelog. OTOH that would render the changelog SQL specific, and it would not solve the problem when applying changelos to SQL Aynwhere created by other DBMSs.

IMHO the current PR is the best option for most users. If users actively complain about the solution, we could later add a PR with a new option to switch it off (and reveal the bug again).

Additional Context

N/A

@mkarg mkarg marked this pull request as ready for review July 25, 2023 15:25
@filipelautert filipelautert self-assigned this Jul 26, 2023
@filipelautert filipelautert added TypeEnhancement SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Jul 26, 2023
@filipelautert filipelautert added SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Jul 26, 2023
@filipelautert
Copy link
Collaborator

@mkarg this one is having a test failure for asany as it is not expecting the set view:

Error:    VerifyChangeClassesTest.compareGeneratedSqlWithExpectedSqlForMinimalChangesets:141 Unexpected difference in /home/runner/work/liquibase/liquibase/liquibase-standard/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/createView/asany.sql
Original:
[-- Database: asany
-- Change Parameter: selectQuery=SELECT id, name FROM person WHERE id > 10
-- Change Parameter: viewName=v_person
CREATE VIEW v_person AS SELECT id, name FROM person WHERE id > 10;]
New state:
[-- Database: asany
-- Change Parameter: selectQuery=SELECT id, name FROM person WHERE id > 10
-- Change Parameter: viewName=v_person
SET TEMPORARY OPTION force_view_creation='ON';
CREATE VIEW v_person AS SELECT id, name FROM person WHERE id > 10;]```

…eation of dependent views even if dependency does not yet exist.
@mkarg
Copy link
Contributor Author

mkarg commented Jul 27, 2023

@mkarg this one is having a test failure for asany as it is not expecting the set view:

@filipelautert Oops, sorry, seems I missed to push one changed file. Uploaded it a minute ago. Good catch, thank you! :-)

@filipelautert filipelautert added SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Jul 27, 2023
Copy link
Collaborator

@filipelautert filipelautert left a comment

Choose a reason for hiding this comment

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

SQL anywhere specific changes.

@filipelautert filipelautert added this to the 1NEXT milestone Aug 29, 2023
@filipelautert filipelautert merged commit ce523cc into liquibase:master Aug 29, 2023
32 of 35 checks passed
@mkarg mkarg deleted the issue-4536 branch September 11, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions TypeEnhancement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

SQL Anywhere fails to create view-on-a-view if changes are performed in wrong order
4 participants