Skip to content

refactor(connectors): split PEM and EXPLAIN code out of connector index files - #467

Merged
tianzhou merged 1 commit into
mainfrom
claude/bold-albattani-4fm1yh
Oct 9, 2026
Merged

tianzhou merged 1 commit into
mainfrom
claude/bold-albattani-4fm1yh

Conversation

@tianzhou

@tianzhou tianzhou commented Oct 9, 2026

Copy link
Copy Markdown
Member

Summary

Pure code moves out of the two largest connector files. No behaviour change.

  • postgres: the sslrootcert/sslcert/sslkey handling (PEM reading, encrypted-key detection, SSL mode constants, and the pg.Client subclass that re-reads PEMs on every connection) moves from index.ts to ssl-pems.ts, alongside the existing failed-to-read-certificate.ts and type-parsers.ts helpers.
  • sqlserver: the EXPLAIN emulation (SHOWPLAN_XML / STATISTICS XML on a dedicated single-connection pool, option prefix parsing, rollback wrapper, plan extraction) moves to explain.ts. The functions take the connector's sql.config as a parameter instead of reading this.config.
  • sqlserver: bindParameters and assertNoReadOnlyEscapes, shared by executeReadOnly and the explain paths, move to request-helpers.ts so both files import them from one place.
  • The executeSQL guard in the SQL Server connector now checks config alongside connection instead of using a non-null assertion at the explain dispatch. Both are always set together in connect.
File Before After
postgres/index.ts 1073 944
sqlserver/index.ts 1397 980

Test plan

  • pnpm run build succeeds
  • pnpm test:unit passes (1201 tests)
  • tsc --noEmit output identical to main (pre-existing errors only, none in touched files)
  • Moved bodies diffed against the originals, normalised for indentation; only the signature changes differ
  • CI integration suites for SQL Server (EXPLAIN coverage) and Postgres client-cert, which need Docker and could not run locally

🤖 Generated with Claude Code

https://claude.ai/code/session_01T79tgGfqWNqUeTwhRc65MJ


Generated by Claude Code

…ex files

Pure code moves, no behaviour change.

postgres: the sslrootcert/sslcert/sslkey handling (PEM reading, encrypted
key detection, and the pg.Client subclass that re-reads PEMs on every
connection) moves from index.ts to ssl-pems.ts, alongside the existing
failed-to-read-certificate.ts and type-parsers.ts helpers.

sqlserver: the EXPLAIN emulation (SHOWPLAN_XML / STATISTICS XML on a
dedicated single-connection pool, option prefix parsing, rollback wrapper
and plan extraction) moves to explain.ts, taking the connector's config
as a parameter instead of reading this.config. bindParameters and
assertNoReadOnlyEscapes, shared by executeReadOnly and the explain paths,
move to request-helpers.ts so both files import them from one place.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01T79tgGfqWNqUeTwhRc65MJ
Copilot AI balanced review requested due to automatic review settings October 9, 2026 09:16

Copilot AI 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.

🟢 Approval recommended

The extracted logic preserves existing behavior and remains covered by targeted connector tests.

0 open findings

What changed in this PR

Refactors PostgreSQL PEM handling and SQL Server EXPLAIN logic into focused modules without changing intended behavior.

Changes:

  • Extracts PostgreSQL certificate loading and rotation helpers.
  • Extracts SQL Server EXPLAIN emulation and shared request helpers.
  • Strengthens SQL Server connection/configuration guarding.
File Description
src/​connectors/​postgres/​index.ts Uses extracted PEM utilities.
src/​connectors/​postgres/​ssl-pems.ts Houses PostgreSQL PEM handling.
src/​connectors/​sqlserver/​index.ts Delegates EXPLAIN and request operations.
src/​connectors/​sqlserver/​explain.ts Houses SQL Server EXPLAIN emulation.
src/​connectors/​sqlserver/​request-helpers.ts Centralizes parameter binding and read-only checks.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@tianzhou
tianzhou merged commit e80f778 into main Oct 9, 2026
3 checks passed
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.

3 participants