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

[9.x] Query PostgresBuilder fixes for renamed config 'search_path' #41215

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

derekmd
Copy link
Contributor

@derekmd derekmd commented Feb 24, 2022

Fixes #41213 and more

  1. PostgresBuilder::parseSchemaAndTable()
    • [9.x] Fix postgresql build #39531 missed one config('database.connections.pgsql.search_path') fallback to <= 8.x key 'database.connections.pgsql.schema'.
    • Remove duplicate $user schema variable replacement already done by parseSearchPath() that this method calls.
  2. PostgresBuilder::getAllTables() + getAllViews() + parseSchemaAndTable()
    • [9.x] Fix parsing config('database.connections.pgsql.search_path') #41088 fixed valid schemas like 'test-db' not being parsed. These 3 methods still have that problem.
    • I missed parseSearchPath() was copy & pasted between PostgresConnector and PostgresBuilder with a one-line difference for $user replacements.
    • Introduce a shared ParsesSearchPath trait to fix PostgresBuilder methods not handling all search paths.
  3. DatabasePostgresBuilderTest
    • Add more test cases for <= 8.x fallback config('database.connections.pgsql.schema') and cover an array of search paths with non-alphanumeric strings containing hyphen or accented characters.
    • Class cleanup - extensive comments and long method names can be concisely communicated with terse method naming.

@derekmd derekmd force-pushed the fix-postgres-builder-search-path branch from 0704627 to 557908c Compare February 24, 2022 04:10
@derekmd derekmd changed the title [9.x] Query PostgresBuilder fixes for config('database.connections.pgsql.search_path') rename [9.x] Query PostgresBuilder fixes for renamed config 'search_path' Feb 24, 2022
….search_path')`

1. `PostgresBuilder::parseSchemaAndTable()`
   * Needs a fallback to <= 8.x `config('database.connections.pgsql.schema')`
     when 9.x renamed `config('database.connections.pgsql.search_path')`
     is missing.
   * Remove duplicate `$user` variable `config('database.connections.pgsql.username')`
     replacement handling already done by `parseSearchPath()`.
2. `PostgresBuilder::getAllTables()` + `getAllViews()` + `parseSchemaAndTable()`
   Apply the `parseSearchPath()` fixes applied to PostgresConnector:
   laravel#41088
3. `DatabasePostgresBuilderTest`
   Add more test cases and use terse method names instead of extensive
   comments to concisely communicate each case.
@derekmd derekmd force-pushed the fix-postgres-builder-search-path branch from 557908c to fa8a17b Compare February 24, 2022 04:16
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Thanks @derekmd

@taylorotwell taylorotwell merged commit bbf50f4 into laravel:9.x Feb 24, 2022
@taylorotwell
Copy link
Member

❤️

@derekmd derekmd deleted the fix-postgres-builder-search-path branch February 24, 2022 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong parse of search path for table in different schemas of PostgreSQL Builder.
3 participants