Skip to content

feat: support PG connection properties in driver#886

Open
surbhigarg92 wants to merge 1 commit into
mainfrom
pg_connection_properties
Open

feat: support PG connection properties in driver#886
surbhigarg92 wants to merge 1 commit into
mainfrom
pg_connection_properties

Conversation

@surbhigarg92

@surbhigarg92 surbhigarg92 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds support for PostgreSQL-specific GUC configuration properties (application_name, timezone, client_min_messages, client_encoding, default_transaction_isolation, server_version, and server_version_num). These defaults and no-op properties enable standard PostgreSQL drivers to connect and execute initial setup queries seamlessly.

Changes

  • PostgreSQL GUC Properties: Defined read-only and no-op PostgreSQL compatibility properties.
  • Dialect Isolation: Segregated PostgreSQL properties from GoogleSQL connections so GoogleSQL connection states remain clean.
  • Performance Optimization: Pre-computed postgresConnectionProperties during package init() to avoid map allocations and copying on every connection setup.
  • Test Coverage: Added mock server unit tests in conn_with_mockserver_test.go verifying read/write behavior, read-only validation, and dialect restriction.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces PostgreSQL compatibility connection properties (GUCs) and corresponding mock server tests for the Spanner PostgreSQL dialect. The feedback suggests optimizing performance by pre-computing the merged PostgreSQL connection properties map during package initialization, which avoids map allocation and copying overhead on every connection creation.

Comment thread connection_properties.go Outdated
Comment thread connection_properties.go Outdated
@surbhigarg92
surbhigarg92 force-pushed the pg_connection_properties branch from 65c870d to cadca3f Compare July 23, 2026 06:37
@surbhigarg92
surbhigarg92 marked this pull request as ready for review July 23, 2026 06:39
@surbhigarg92
surbhigarg92 requested a review from a team as a code owner July 23, 2026 06:39
@surbhigarg92

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces PostgreSQL compatibility connection properties (GUCs) such as application_name, client_min_messages, client_encoding, timezone, default_transaction_isolation, server_version_num, and server_version for the PostgreSQL dialect, along with corresponding tests. Feedback recommends using Go subtests (t.Run) to isolate distinct test cases in accordance with the repository style guide, and adding missing verifyShowFails assertions to align with the test's documented behavior.

Comment thread conn_with_mockserver_test.go
Comment thread conn_with_mockserver_test.go
@surbhigarg92
surbhigarg92 force-pushed the pg_connection_properties branch from cadca3f to 6c724cf Compare July 23, 2026 07:03
@surbhigarg92

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request adds PostgreSQL compatibility connection properties (GUCs) such as application_name, client_min_messages, client_encoding, timezone, default_transaction_isolation, server_version_num, and server_version to support client library setup queries when using the PostgreSQL dialect. The review feedback suggests making default_transaction_isolation writable (validating that it is set to serializable) instead of strictly read-only, as standard PostgreSQL drivers or ORMs may attempt to set this parameter during initialization. Additionally, the feedback recommends adding a test case to verify that setting it to 'serializable' succeeds and updating the corresponding subtest name.

Comment thread connection_properties.go Outdated
Comment thread conn_with_mockserver_test.go
Comment thread conn_with_mockserver_test.go Outdated
@surbhigarg92

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces PostgreSQL compatibility connection properties (GUCs) such as application_name, client_min_messages, client_encoding, timezone, default_transaction_isolation, server_version_num, and server_version to support client library setup queries when using the PostgreSQL dialect. It also adds corresponding unit tests. The review feedback correctly identifies that the newly acquired database connections in the tests are not closed, which could lead to connection leaks, and suggests using defer silentClose(conn) to resolve this.

Comment thread conn_with_mockserver_test.go
Comment thread conn_with_mockserver_test.go
@surbhigarg92
surbhigarg92 force-pushed the pg_connection_properties branch from 6c724cf to c9bd260 Compare July 23, 2026 07:26
@surbhigarg92
surbhigarg92 force-pushed the pg_connection_properties branch from c9bd260 to d5efdbe Compare July 23, 2026 07:52
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