Skip to content

Honor explicit default_query_exec_mode; no forced simple_protocol#144

Open
btipling wants to merge 2 commits into
mainfrom
bt/postgres-simple-protocol-default
Open

Honor explicit default_query_exec_mode; no forced simple_protocol#144
btipling wants to merge 2 commits into
mainfrom
bt/postgres-simple-protocol-default

Conversation

@btipling

@btipling btipling commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Postgres/pgx defaults to prepared-statement caching (cache_statement). Transaction-mode poolers (PgBouncer, Supabase pooler on 6543) need default_query_exec_mode=simple_protocol to avoid SQLSTATE 42P05 (prepared statement "stmtcache_*" already exists).

This PR is non-breaking. baton-sql does not inject simple_protocol when the mode is unset. Callers that need pooler-safe behavior set the param explicitly.

Config state Behavior
default_query_exec_mode unset (no DSN query key, no connect.params key) Leave URL as-is; pgx uses its default
Explicit value in DSN or connect.params Honored (simple_protocol, cache_statement, exec, etc.)

Pooler opt-in

connect:
  dsn: "postgres://${HOST}:6543/${DB}"
  params:
    default_query_exec_mode: simple_protocol

Or via DSN: ?default_query_exec_mode=simple_protocol.

Changes

File Change
pkg/database/database.go No inject of simple_protocol when unset (removed forced default)
pkg/database/database_test.go Unset → no mode key; explicit DSN/params preserved; mysql unchanged
README.md Document pooler opt-in (not forced default)
examples/redshift-test.yml Restore opt-in caveat for prepared-statement quirks

Tests

go test ./pkg/database/...
go test ./...

Both exit 0.

Context

  • Bead: ds-n4f4
  • C1 connector-level defaults remain out of scope for baton-sql

Checklist

  • Unset postgres DSN does not inject default_query_exec_mode
  • Explicit mode in DSN / connect.params still honored
  • Tests + README + redshift example updated
  • Unit tests green

Inject default_query_exec_mode=simple_protocol for postgres scheme
URLs in buildConnectionURL so transaction-mode poolers (PgBouncer,
Supabase) do not hit SQLSTATE 42P05 prepared-statement conflicts.
Explicit DSN/Params values still win; non-postgres schemes unchanged.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
@btipling
btipling requested a review from a team July 27, 2026 01:38
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: Honor explicit default_query_exec_mode; no forced simple_protocol

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 9e81038.
Review mode: incremental since e4c96bf
View review run

Review Summary
The full PR diff was scanned for security and correctness. The new commit f19600e removes the forced simple_protocol injection that existed at the previously-reviewed SHA, so pkg/database/database.go is now net-unchanged from main; the PR only touches tests, README.md, and the redshift example. This addresses the prior concern that force-injecting default_query_exec_mode was a breaking behavior change for existing postgres connectors relying on the pgx cache_statement default; explicit DSN and connect.params values remain honored via the existing params loop, and the new test Test_buildConnectionURL_PostgresQueryExecMode covers unset, explicit, and mysql cases. No new issues found.

Security Issues: None found.

Correctness Issues: None found.

Suggestions: None.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

Forced injection broke existing postgres connectors that rely on pgx
default cache_statement. Leave the URL unchanged when unset; honor
explicit DSN or connect.params values (including simple_protocol for
poolers). Update tests, README pooler opt-in guidance, and redshift
example caveat.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
@btipling btipling changed the title Default postgres connections to simple_protocol Honor explicit default_query_exec_mode; no forced simple_protocol Jul 27, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

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.

1 participant