fix(pg,gel): forward the selection argument through withReplicas $with - #6198
Open
pri12ya871 wants to merge 1 commit into
Open
fix(pg,gel): forward the selection argument through withReplicas $with#6198pri12ya871 wants to merge 1 commit into
pri12ya871 wants to merge 1 commit into
Conversation
$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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
$withtakes(alias, selection?), but thewithReplicaswrapper forwarded only the first argument:Every other method on that wrapper spreads its arguments —
select,selectDistinct,$count,with—$withalone did not. So the documented "CTE from raw SQL with a declared selection" pattern silently lost its selection underwithReplicas, and every field read off the CTE came backundefined.This is the first of the three problems @shadoworion reported in #6187. I have deliberately left the other two alone —
with recursivesupport and array parsing for unboundsqlfields 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-coreis where it was reported.gel-corecarries the identical line and is fixed alongside it. Themysql,sqliteandsinglestorewrappers name a$withvariable but forwardwith, so they are a different shape and untouched here.Test
integration-tests/tests/replicas/postgres.test.tsgains a test in the existingdrizzle.mock()style, so it needs no database. It asserts three things: that the replica's$withis called with both arguments, that a field on the resulting CTE is defined rather thanundefined, 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:
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.tsexits during the zx-driven steps), andpnpm installat the root fails on adrizzle-kit@0.25.0-b1faa33devDependency that is no longer published. I installed with--filter drizzle-orm --filter integration-testsand ran the replica tests againstdrizzle-orm/srcthrough 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.