fix: extract postgres commons for watt - #1309
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the internal PostgreSQL “common” utilities out of the direct adapter implementation into src/internal/database/postgres/*, making them reusable by both the direct and Watt database adapters (per the target database/postgres, database/direct, database/watt split referenced in the description).
Changes:
- Moved/centralized Postgres helper logic (SQL helpers, scope SQL builder, SSL settings parsing, query cancellation, pool error handling, abort-signal assertions, and type parser install) into
src/internal/database/postgres/*. - Updated internal call sites to import from the new
postgres/*modules and adjusted database index exports accordingly. - Added focused Vitest coverage for each extracted helper module and updated existing pg-connection tests to match the new structure.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal/testing/seeder/pg-persistence.ts | Updates quoteIdentifier import to the extracted Postgres SQL helpers. |
| src/internal/queue/database.ts | Re-exports quoteIdentifier from the new Postgres SQL module for queue DB usage. |
| src/internal/database/tenant-store-pg.ts | Switches quoteIdentifier import to ./postgres/sql. |
| src/internal/database/sql.ts | Removes the old internal SQL helper module (moved to postgres/sql.ts). |
| src/internal/database/sql.test.ts | Removes the old SQL helper tests (replaced by postgres/sql.test.ts). |
| src/internal/database/postgres/type-parsers.ts | Adds a shared installer for the global int8 text parser. |
| src/internal/database/postgres/type-parsers.test.ts | Adds tests validating int8 parser installation behavior. |
| src/internal/database/postgres/ssl.ts | Introduces shared SSL settings logic (SecureContext caching + hostname/IP handling). |
| src/internal/database/postgres/ssl.test.ts | Updates tests to import from the new ssl module and validates edge cases. |
| src/internal/database/postgres/sql.ts | Adds extracted SQL utilities (identifier quoting, statement normalization, BEGIN mode builder, timeout normalization). |
| src/internal/database/postgres/sql.test.ts | Adds test coverage for the extracted SQL utilities. |
| src/internal/database/postgres/scope.ts | Adds a shared builder for session scope set_config(...) SQL + placeholders. |
| src/internal/database/postgres/scope.test.ts | Adds coverage for placeholder ordering and optional settings behavior. |
| src/internal/database/postgres/pool-errors.ts | Extracts pool error classification, disposable-client marking, and error handler attachment. |
| src/internal/database/postgres/pool-errors.test.ts | Adds tests for pool error helpers and AbortError creation/marking. |
| src/internal/database/postgres/cancellation.ts | Extracts query cancellation mechanics and cancel-target derivation. |
| src/internal/database/postgres/cancellation.test.ts | Adds tests for cancel-target derivation rules. |
| src/internal/database/postgres/asserts.ts | Extracts abort-signal validation logic used by query execution. |
| src/internal/database/postgres/asserts.test.ts | Adds coverage for abort-signal assertion behavior. |
| src/internal/database/pg-connection.ts | Refactors the direct adapter to consume extracted Postgres helper modules. |
| src/internal/database/pg-connection.test.ts | Updates tests for removed cancel-target helper and adds a check for global int8 parser install. |
| src/internal/database/multitenant-pg.ts | Switches pool error handling to the extracted attachPoolErrorHandler. |
| src/internal/database/migrations/migrate.ts | Updates SSL helper import to ../postgres/ssl. |
| src/internal/database/migration-admin-store-pg.ts | Switches quoteIdentifier import to ./postgres/sql. |
| src/internal/database/index.ts | Re-exports extracted Postgres helpers from the database internal index. |
| src/http/plugins/vector.ts | Updates vector pool error handling to use attachPoolErrorHandler and logs directly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage Report for CI Build 31372021862Coverage increased (+0.009%) to 80.76%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
There was a problem hiding this comment.
This is a large, purely mechanical refactor (25 files) that extracts postgres primitives (SQL helpers, scope statement building, cancellation, pool-error classification, SSL, type parsers) into new postgres/* modules, but it touches core query/transaction/cancellation logic in pg-connection.ts that is on the critical path for every DB call. I didn't find behavioral differences in the extracted code, but given the criticality of this path I think it's worth a human sanity-check rather than a bot-only approval.
Beyond the inline findings (none), I also checked the barrel omission raised by the bug hunter: src/internal/database/index.ts doesn't re-export the new postgres/asserts, postgres/cancellation, postgres/scope, postgres/ssl, and postgres/type-parsers modules. I confirmed no call site imports these from the @internal/database barrel — all consumers (pg-connection.ts, migrations/migrate.ts) import them directly from their ./postgres/* paths, so this omission is not a live bug.
Extended reasoning...
One short paragraph as instructed for the ruled-out-note case: verified via grep that getSslSettings, installPostgresTypeParsers, buildScopeStatement, cancelQuery, and assertValidSignal are only ever imported directly from their postgres/* module paths (in pg-connection.ts and migrations/migrate.ts), never via the @internal/database barrel, so the barrel's omission of those modules does not break any current caller. Also spot-checked the extracted logic itself (scope SQL placeholder numbering, pool-error classification, cancellation target resolution, BEGIN statement building) against the pre-refactor code in the diff and found it byte-for-byte equivalent, just relocated.
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
fda2cc2 to
5f32573
Compare
What kind of change does this PR introduce?
Refactor
What is the current behavior?
Most of postgres primities are tied to direct implementation.
What is the new behavior?
Extract a package to be shared between direct and watt.
Final shape:
Additional context
Related to #1230