Skip to content

Handle pg_catalog.set_config session state#1069

Closed
erayack wants to merge 1 commit into
pgdogdev:mainfrom
erayack:issue-1055-set-config-core
Closed

Handle pg_catalog.set_config session state#1069
erayack wants to merge 1 commit into
pgdogdev:mainfrom
erayack:issue-1055-set-config-core

Conversation

@erayack

@erayack erayack commented Jun 12, 2026

Copy link
Copy Markdown

Summary

Fixes PgDog session-state tracking for pg_catalog.set_config(...), which is used by tools like pg_dump to mutate settings such as search_path.

Before this change, PgDog forwarded:

SELECT pg_catalog.set_config('search_path', '...', false)

to PostgreSQL normally, but did not update the client-side Parameters state. If the client later received a different pooled backend connection, PgDog would not replay the
updated setting, so queries could run with a stale search_path.

This PR:

  • recognizes simple top-level SELECT pg_catalog.set_config(, , )
  • also supports the existing unqualified SELECT set_config(...) form for backward compatibility
  • adds a dedicated Command::SetConfig
  • executes the SQL normally so clients still receive PostgreSQL’s SELECT result row
  • applies the parameter update only after PostgreSQL sends CommandComplete
  • clears pending state on backend error
  • reuses the existing SetParam / Parameters transaction machinery
  • routes dynamic or untracked set_config(...) calls as writes

Semantics

set_config(name, value, false) is tracked like session SET:

  • outside a transaction: persists in client params and is replayed on future backend reuse
  • inside a transaction: stored as transaction params and committed/rolled back with the transaction

set_config(name, value, true) is tracked like SET LOCAL:

  • inside a transaction: visible for the transaction only
  • outside a transaction: not persisted in PgDog client params

Unsupported forms remain normal queries, for example:

  SELECT set_config('search_path', current_setting('search_path'), false);
  SELECT set_config($1, $2, false);
  SELECT set_config(name, value, false) FROM settings;

Tests

  • cargo fmt
  • cargo test -p pgdog frontend::router::parser::query::test::test_set::test_set_config --no-fail-fast

Added coverage for:

  • parser recognition of pg_catalog.set_config(...)
  • parser recognition of unqualified set_config(...)
  • is_local = true
  • dynamic arguments remaining normal write queries
  • complex/multi-statement SELECTs remaining normal write queries
  • SQLx integration coverage for backend reuse
  • SQLx rollback coverage for transaction-scoped state

Fixes #1055

@CLAassistant

CLAassistant commented Jun 12, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@erayack

erayack commented Jun 12, 2026

Copy link
Copy Markdown
Author

Stacked follow-up for incremental review: https://github.com/erayack/pgdog/pull/1

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2b7f5e15d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +265 to +268
Some(SetParam {
name: Self::string_const(&func.args[0])?,
value: ParameterValue::String(Self::string_const(&func.args[1])?),
local: Self::bool_const(&func.args[2])?,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve multi-entry search_path values

When set_config is used for a multi-entry search_path, e.g. SELECT set_config('search_path', 'public, pg_catalog', false), this stores the whole second argument as a single ParameterValue::String. On the next backend checkout PgDog replays tracked params via SET "search_path" TO {value}, so that value becomes SET "search_path" TO "public, pg_catalog", which PostgreSQL interprets as one quoted schema name rather than the two schemas that were actually set. This breaks session-state replay for clients that use set_config with normal comma-separated search paths.

Useful? React with 👍 / 👎.

@sgrif

sgrif commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Thank you for your submission. I apologize for not assigning myself to the issue sooner. Unfortunately this pull request overlaps with work the pgdog staff already had in progress. Duplicate of #1072

@sgrif sgrif closed this Jun 12, 2026
@erayack

erayack commented Jun 12, 2026

Copy link
Copy Markdown
Author

No worries, thanks for letting me know. If any part of this implementation or the tests are useful, you’re welcome to cherry-pick or copy them.

Thank you for your submission. I apologize for not assigning myself to the issue sooner. Unfortunately this pull request overlaps with work the pgdog staff already had in progress. Duplicate of #1072

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.

Handle pg_catalog.set_config

3 participants