Skip to content

Run one builder suite on every dialect; cover the Postgres entry - #5

Merged
Makisuo merged 2 commits into
chore/rename-effect-ormfrom
test/pg-dialect-coverage
Oct 3, 2026
Merged

Makisuo merged 2 commits into
chore/rename-effect-ormfrom
test/pg-dialect-coverage

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #4.

Borrowed from how drizzle tests its dialects: one shared suite run against every database, with named, reasoned skips. On top of that, the coverage manifest we already had for ClickHouse now also covers the shared builder and the Postgres entry.

Shared builder suite (tests/core-cases.ts)

  • 20 cases cover selection, every shared operator, params, group/having, limit/offset, all six join kinds, CTEs, unions, route/crossTenant and format.
  • Both databases read the same fixture rows from a WITH (values(...) on ClickHouse, VALUES on Postgres), so the suite writes nothing.
  • It runs live on ClickHouse under both join_use_nulls settings, and on PGlite on every vitest run. core-sql.test.ts snapshots the SQL and parameters for each dialect.
  • Real differences between the databases are recorded per target: / is integer division on Postgres, and ClickHouse fills a missing join row with defaults when join_use_nulls=0. rejects covers clauses a dialect refuses (format on Postgres).

Manifests (tests/dialect-coverage.test.ts)

  • Every query/union method, expression/condition operator and param kind needs a core case.
  • Every ./postgres export needs a case in dialect-cases.postgres.ts, or an exemption with a reason.

Bugs it found, fixed in the first commit

  • Postgres: a UNION ALL branch with its own WITH/ORDER BY/LIMIT was a syntax error. Branches are now parenthesized (clauses.parenthesizeUnionBranches).
  • Postgres: param.float compared with an int8 column was bound as int8, so 19.5 was rejected. A param.bool in a select list was bound as text. Float, bool and timestamp params now bind with a cast. string/int stay uncast so they still compare with enum and int4 columns.
  • Docs: custom("int8", Schema.String) for exact int8 fails on drivers that send bigint (PGlite, postgres.js).

Verified: typecheck; bun run test (427 passed, live-only suites skipped); the full suite against ClickHouse 26.8.2.7 in Docker (593 passed, 0 skipped).

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Makisuo and others added 2 commits October 3, 2026 16:01
Each UNION ALL branch is now parenthesized on Postgres, where a branch with
its own WITH, ORDER BY or LIMIT was a syntax error. The placeholder gets the
param kind, so float, bool and timestamp params bind with a cast instead of
taking an int8 or text type from their context. The exact-int8 recipe now
reads a bigint too, which PGlite and postgres.js send.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
core-cases.ts runs the shared builder surface (query and union methods,
operators, params) on live ClickHouse and on PGlite, pins the SQL per dialect
in snapshots, and records real differences per target. The coverage manifest
now requires a core case for every shared method and a Postgres case for every
./postgres export, as it already did for the ClickHouse catalog.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bccb84dd-cf99-4314-a419-f36a33c823ba

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Makisuo
Makisuo merged commit eb865b4 into chore/rename-effect-orm Oct 3, 2026
4 checks passed
@Makisuo
Makisuo deleted the test/pg-dialect-coverage branch October 3, 2026 15:34
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