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

Ensure Postgresql search_path entries are quoted correctly #3009

Merged
merged 1 commit into from Jul 12, 2022

Conversation

nvoxland
Copy link
Contributor

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

In postgresql and it's varients (redshift, cockroachdb, etc), the search_path that gets set based on the defaultSchemaName uses the value returned from show search_path. Sometimes, objects such as $user in that shown search path are note quoted like is needed to re-set them.

This PR updates the logic from handling only values that start with a $ and only on cockroachdb to handling all non-quoted values on all postgresql versions.

The change handles cases where a value comes back already quoted from show search_path.

For example, if defaultSchemaName is being set to test, it will set search_path=test, "$user", "public" whether the original show seach_path returned "$user", public or $user, public.

By quoting "public" like that, it will enforce the case on public, but in my testing the value in search_path was already correctly cased. If I called set search_path=$user, PUBLIC" then show search_path still returned $user, public. And if I called set search_path=$user, "TEST", public it showed as "$user", "TEST", public. So the change seems correct still.

Fixes #2928

Things to be aware of

  • Nothing beyond what is in description

Things to worry about

  • Nothing

@github-actions
Copy link

github-actions bot commented Jun 27, 2022

Unit Test Results

  4 548 files  ±0    4 548 suites  ±0   31m 45s ⏱️ + 1m 33s
  4 524 tests ±0    4 310 ✔️ +4     214 💤  - 4  0 ±0 
53 580 runs  ±0  48 572 ✔️ +4  5 008 💤  - 4  0 ±0 

Results for commit c26ae49. ± Comparison against base commit 3f5230d.

♻️ This comment has been updated with latest results.

@ecarneiro01 ecarneiro01 self-assigned this Jun 28, 2022
@ecarneiro01
Copy link

I checked that the generated SQL of update-sql could be applied on the databases and that the object $user was being quoted correctly:

Postgres:

  • Use default values for the search path and apply changes to the DB. PASS
  • Use defaultSchemaName property and apply changes to the DB. PASS

Redshift:

  • Use default values for the search path and apply changes to the DB. PASS
  • Use defaultSchemaName property and apply changes to the DB. PASS

CockroachDB:

  • Use default values for the search path and apply changes to the DB. PASS
  • Use defaultSchemaName property and apply changes to the DB. PASS

@kataggart kataggart added this to To Do in Conditioning++ via automation Jul 12, 2022
@kataggart kataggart added this to the NEXT milestone Jul 12, 2022
@nvoxland nvoxland merged commit 3bf2706 into master Jul 12, 2022
Conditioning++ automation moved this from To Do to Done Jul 12, 2022
@nvoxland nvoxland deleted the pg-searchpath-parsing branch July 12, 2022 15:42
@fatso83
Copy link

fatso83 commented Nov 8, 2023

This was a breaking change (#3205) for a lot of people, but was not marked as such in the release notes 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

redshift does not work with 4.11 release
5 participants