Pin the output settings on every ACE connection

[danolivo]

Problem

ACE compares rows by the text that each server prints for them. table-diff and mtree hash the text of each column, and the CDC walsender sends pgoutput values as text. This text depends on session settings: TimeZone,
DateStyle, IntervalStyle, extra_float_digits, bytea_output, lc_monetary. ACE used the defaults of each server. These defaults come from postgresql.conf, ALTER DATABASE ... SET and ALTER ROLE ... SET, so they can differ between nodes. Then equal rows give different hashes, and ACE reports differences that do not exist. The most common case is a timestamptz column on nodes with different TimeZone.

ACE also did not set client_encoding. The server then sends text in the database encoding, but Go strings and pgx expect UTF-8.

Logical replication solves a part of this problem: the subscriber connects with its own client_encoding and with datestyle=ISO, intervalstyle=postgres and extra_float_digits=3 (see libpqwalreceiver.c). This is enough for logical replication, because the subscriber only reads each value back. ACE compares the text byte by byte, so it needs more settings, with fixed values.

Changes

• Every ACE connection, the pool and the replication connection, sends these values in its startup packet:

  Setting	Value
  client_encoding	UTF8
  DateStyle	ISO, MDY
  IntervalStyle	postgres
  TimeZone	UTC
  extra_float_digits	3
  bytea_output	hex
  lc_monetary	C
  standard_conforming_strings	on

A value in the startup packet has priority over role and database defaults and over PGOPTIONS. It costs no extra round trip, and it also covers the replication connection, which has no AfterConnect hook. Keys that differ only in case (for example TimeZone from a connection string, or timezone from PGTZ) are removed, so each setting is sent once.

• A connection pooler can drop a startup parameter without an error. A ValidateConnect hook compares the values that the server reports in ParameterStatus (client_encoding, DateStyle, IntervalStyle, TimeZone, standard_conforming_strings) with the values that ACE sent. If one of them differs, the connection fails with an error that names the setting. An existing hook, for example from target_session_attrs, still runs first.
• schema-diff builds its SET LOCAL statements from the same list (auth.OutputSettings()), and adds search_path = pg_catalog. So the two places cannot get different values.
• CurrentHashVersion is now 3. The next mtree update recomputes all stored hashes of a tree once.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 41fde33a-1a28-4018-8e46-4f6603f9682b

📥 Commits

Reviewing files that changed from the base of the PR and between 557892d and b9802ff.

📒 Files selected for processing (10)

• db/queries/queries.go
• docs/CHANGELOG.md
• internal/consistency/schema/collect.go
• internal/consistency/schema/collect_test.go
• internal/infra/db/auth.go
• internal/infra/db/auth_test.go
• tests/integration/connection_settings_test.go
• tests/integration/crash_recovery_test.go
• tests/integration/recovery_single_node_test.go
• tests/integration/service_wait_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

ACE pool and replication connections now pin and validate output settings. Schema deparsing uses those settings, and mtree hash version 3 accounts for them. Integration recovery tests add service-readiness and replication-settling checks.

Changes

Session output consistency

Layer / File(s)	Summary
Pin and validate connection settings internal/infra/db/auth.go, internal/infra/db/auth_test.go	Pool and replication configurations apply fixed output settings and validate reported values. Tests cover parameter replacement, configuration, and validation.
Use settings for deparsing and hash versioning db/queries/queries.go, internal/consistency/schema/collect.go, internal/consistency/schema/collect_test.go	Schema deparsing builds local settings from connection output settings, excluding client encoding. The mtree hash version increases to 3, and a test checks the deparse statements.
Verify connection output and document changes tests/integration/connection_settings_test.go, docs/CHANGELOG.md	Integration tests check settings and formatted values on pool and replication connections. The changelog records the settings and hash-version update.

Integration recovery test readiness

Layer / File(s)	Summary
Add service and replication wait helpers tests/integration/service_wait_test.go	Helpers poll for query readiness and check subscription state and replication-slot positions until settled or timed out.
Apply readiness checks in recovery tests tests/integration/crash_recovery_test.go, tests/integration/recovery_single_node_test.go	Recovery test setup and cleanup wait for service readiness and replication settling, and report restart, subscription, and table-drop errors.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to b9802

The identified lint changes are not required by this repository, and the checked connection and recovery-wait paths show no actionable failure. The PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b9802

Pinning connection settings improves comparison consistency. The remaining risk is rollback: an older release can reuse newly generated comparison data without checking compatibility.

Retained concerns

• Medium · reliability · inferred: After a tree is rebuilt with version 3, reverting to the version-2 binary does not reject or rebuild it: the consumer upgrades only when the stored version is lower. Subsequent dirty-block updates can therefore combine server-default hashes with remaining pinned-setting hashes while retaining the version-3 marker. This undermines compatibility and comparison guarantees when those settings produce different text. The acceptance mechanism predates this PR, but the PR introduces the version-3 state that exposes this rollback path. Supported downgrade policy is not established.

Security review details

Security Blast Radius

• inferred — The material exposure is the tables, stored comparison trees, and replication streams accessed through existing ACE database connections. The inspected production changes alter formatting and compatibility rather than introducing a new remote entrypoint or requesting additional database authority. Replication-slot creation remains an existing privileged operation.

Trust Boundaries and Controls

• observed — Configuration-derived case variants cannot override pinned output parameters. Validation checks server-reported formatting values while preserving prior connection controls. It is not an independent server-identity control, and extra_float_digits, bytea_output, and lc_monetary are explicitly outside startup-report validation.

Resilience and Maintainability Implications

• observed — Integration coverage checks all eight settings on pool and replication connections after changing role defaults. Unit coverage checks missing and mismatched reports and preservation of an existing validation hook. These are source-level test assertions; production pooler behavior and successful test execution were not observed.

Hardening Proposals

• proposed — Define rollback and mixed-version operation explicitly. Require compatible stored hash versions before comparison, and provide a documented rebuild or invalidation procedure when returning to an older release. Exercise interruption, repetition, and mixed-version writers against that contract.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 9 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: pinning output settings on every ACE connection.
Description check	✅ Passed	The description explains the problem, the connection settings, validation, schema-diff changes, and hash version update.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 9 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{A rabbit checks the clock set to UTC,
Then checks each bytea, float, and tree.
The dates line up; the hashes agree,
While sleepy nodes catch up carefully.
I nibble the changelog, pleased as can be.}

_{Comment @coderabbitai help to get the list of available commands.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 13 complexity · 0 duplication

Metric	Results
Complexity	13
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}