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

Prevent spurious SET SEARCH_PATH SQL statements for Postgres during update-sql command. Fixes #5316 #5774

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mpvvliet
Copy link
Contributor

@mpvvliet mpvvliet commented Apr 5, 2024

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

Prevent resetting the search path in Postgres after a rollback if we are running in a mode that does not update the database to avoid spurious SET SEARCH_PATH SQL statements.

The logic in DatabaseUtils.initializeDatabase for Postgres expects changes to the SEARCH_PATH to be persisted to detect when the SEARCH_PATH is already correct(ed). However in update-sql mode these changes are not executed, causing the detection mechanism to fail and resulting in extra SET SEARCH_PATH statements.

Things to be aware of

I'm aware of the other change in review that will fix this issue, but that is a bigger change and might take a long time to be merged. This could be a quick win.

Things to worry about

Additional Context

… running in a mode that does not update the database to avoid spurious SET SEARCH_PATH SQL statements
@filipelautert
Copy link
Collaborator

Hi @mpvvliet ! Your PR handles the same as #5444 and also fixes #5316 just aiming at update-sql and does not modifies the user database settings using alter database session. What do you think about the solution proposed in the other PR?

@mpvvliet
Copy link
Contributor Author

mpvvliet commented Apr 6, 2024

@filipelautert I like the solution in the PR #5444 because it avoids the need to re-apply the SEARCH_PATH changes. However since that PR is bigger and seems stuck, I proposed this smaller one.

Happy to close this one if the other one has a shot of getting finalised soon.

@filipelautert
Copy link
Collaborator

AS PR #5444 still pending some tests let's move this one ahead, and if it gets merged we can revert this one here. Thanks @mpvvliet !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Code Review
Development

Successfully merging this pull request may close these issues.

None yet

3 participants