Skip to content

fix(pg,gel): forward the selection argument through withReplicas $with - #6198

Open
pri12ya871 wants to merge 1 commit into
drizzle-team:mainfrom
pri12ya871:fix/with-replicas-drops-cte-selection
Open

fix(pg,gel): forward the selection argument through withReplicas $with#6198
pri12ya871 wants to merge 1 commit into
drizzle-team:mainfrom
pri12ya871:fix/with-replicas-drops-cte-selection

Conversation

@pri12ya871

Copy link
Copy Markdown

What this fixes

$with takes (alias, selection?), but the withReplicas wrapper forwarded only the first argument:

const $with: Q['$with'] = (arg: any) => getReplica(replicas).$with(arg) as any;

Every other method on that wrapper spreads its arguments — select, selectDistinct, $count, with$with alone did not. So the documented "CTE from raw SQL with a declared selection" pattern silently lost its selection under withReplicas, and every field read off the CTE came back undefined.

This is the first of the three problems @shadoworion reported in #6187. I have deliberately left the other two alone — with recursive support and array parsing for unbound sql fields are separate concerns with their own design questions, and folding them in here would make a one-line fix hard to review. Happy to look at either separately.

Scope

pg-core is where it was reported. gel-core carries the identical line and is fixed alongside it. The mysql, sqlite and singlestore wrappers name a $with variable but forward with, so they are a different shape and untouched here.

Test

integration-tests/tests/replicas/postgres.test.ts gains a test in the existing drizzle.mock() style, so it needs no database. It asserts three things: that the replica's $with is called with both arguments, that a field on the resulting CTE is defined rather than undefined, and that the generated SQL is right.

I confirmed it actually catches the bug rather than merely passing — with the fix reverted it fails with:

expected "$with" to be called with arguments: [ 'sq', { userId: PgSerial{ … } } ]
Received: 'sq'

and the file is 36/36 green with the fix.

One note on how I ran it: drizzle-orm's build script does not complete on my machine (Windows — scripts/build.ts exits during the zx-driven steps), and pnpm install at the root fails on a drizzle-kit@0.25.0-b1faa33 devDependency that is no longer published. I installed with --filter drizzle-orm --filter integration-tests and ran the replica tests against drizzle-orm/src through a local-only vitest alias, which is not part of this PR. So the test is verified, but by that route rather than the standard one — worth a second pair of eyes on CI.

$with takes (alias, selection?), but the withReplicas wrapper forwarded
only the first argument:

  const $with = (arg) => getReplica(replicas).$with(arg)

Every other method on the wrapper spreads its arguments; $with alone did
not. The documented "CTE from raw SQL with a declared selection" pattern
therefore lost its selection silently under withReplicas, and every
field read off the CTE came back undefined.

Reported for pg in drizzle-team#6187. gel-core carries the same line and is fixed
with it; the mysql, sqlite and singlestore wrappers forward `with`
rather than `$with`, so they are not affected.

Refs drizzle-team#6187
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