refactor(plugin-postgresql): tidy search-path and centralize identifier/literal escaping#1546
Merged
Merged
Conversation
…lic is the selected schema
…aping through shared helpers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two related cleanups in the PostgreSQL plugin's SQL generation.
publicentry. When the selected schema ispublic, the search-path builder emittedSET search_path TO "public", public. This collapses it toSET search_path TO "public". Every non-public schema still gets"schema", public, and the quote-doubling injection guard is unchanged."name"with doubled inner quotes) was hand-rolled in ~10 spots acrossPostgreSQLPluginDriver,RedshiftPluginDriver, andCockroachPluginDriver. Those now call the sharedquoteIdentifier(_:)from TableProPluginKit. Duplicated literal escaping ('->''), including afileprivate escapeLiteralForColumns, now routes through the canonicalescapeStringLiteral(_:).Why
A user asked why, after switching to a non-public schema, the SQL editor can still query other schemas unqualified. Investigation confirmed this is correct PostgreSQL behavior, not a bug:
search_pathis a name-resolution order, not an access boundary, and the, publicfallback is the principled choice (dropping it breaks unqualified extension functions/types inpublicand buys zero security). The only real defect was the redundant"public", publictoken, plus the scattered copy-paste of escaping logic that the framework already provides in one audited place.Behavior
escapeStringLiteralalso strips null bytes (the oldescapeLiteralForColumnsdid not). This is a defensive improvement with no practical effect: PostgreSQL identifiers cannot contain null bytes.PostgreSQLSchemaQueries.setSearchPathkeeps its own quoting on purpose. That enum is a standalone, separately unit-tested SQL builder, deliberately decoupled from any driver instance, so it does not depend on the instancequoteIdentifier.Tests
Added
publicSchema()toPostgreSQLSearchPathTestslocking in the no-redundancy output. Existing mixed-case, embedded-quote, and injection tests still pass (they take the non-public branch). The quoting/escaping consolidation is behavior-preserving and covered by existing DDL generation paths.No CHANGELOG or docs entry: no user-facing behavior change.