fix(postgres): inferType returns ::text[] for empty/all-null arrays#6
Open
dnplkndll wants to merge 1 commit into
Open
fix(postgres): inferType returns ::text[] for empty/all-null arrays#6dnplkndll wants to merge 1 commit into
dnplkndll wants to merge 1 commit into
Conversation
Previously `inferType([])`, `inferType([null])`, and `inferType([null, null])`
fell through the array branch (which couldn't determine an inner type) into
the generic object branch and returned `::jsonb` / `::jsonb[]`. Both shapes
are incompatible with `= ANY ($N::jsonb)` on CockroachDB:
ERROR: unsupported comparison operator: _id = ANY $2::JSONB:
op ANY <right> requires array, tuple or subquery on right side
The bug is reachable from the GitHub integration's sync loop. When a
workspace has the App installed but no repositories linked,
`pod-github`'s performSync submits a $in query of shape `[null]` for
the `repository` field; the postgres adapter then emits SQL the
database refuses, and the github pod crash-loops indefinitely on that
workspace. (Stock Postgres is more lenient than CockroachDB and may
silently accept the ::jsonb cast, masking the bug there.)
Fix the array branch in inferType to skip null/undefined when sampling
the inner type, and fall back to `::text[]` (a real SQL array type)
when no inner type can be inferred. `$in`-on-Ref callers already
expect that shape, and ANY accepts it on both Postgres and
CockroachDB.
Existing tests in utils.spec.ts had explicit `// BUG:` comments
acknowledging these three cases — flipped to assert the fixed
behavior, and added a `[null]` case (the exact array shape that
crashed the github pod).
Signed-off-by: Don Kendall <dkendall@ledoweb.com>
0e565d8 to
856c2ae
Compare
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 changed
foundations/server/packages/postgres/src/utils.ts— the array branch ofinferTypeno longer falls through to::jsonbwhen it can't sample an inner type. It now:null/undefinedwhen sampling, and::text[](a real SQL array type) when no inner type can be inferred.foundations/server/packages/postgres/src/__tests__/utils.spec.ts— three pre-existing tests carried// BUG:comments documenting the broken behavior; flipped them to assert the fix, and added a[null]case (the exact array shape that triggered the production crash).Why
The Huly
githubintegration's sync loop crash-loops on CockroachDB whenever a workspace has the GitHub App installed but no repositories linked. The repeating error:Trace:
services/github/pod-github/src/worker.ts(performSync) submitsrepository: { $in: [null, ..._repositories] }. The leadingnullis intentional (matches docs with no per-repo association). When_repositoriesis empty, the value is[null].inferType([null])returned::jsonb: the array branch couldn't sample an inner type and fell through to the catch-all object branch.$inSQL builder then emitted_id = ANY ($N::JSONB). CockroachDB rejects that; stock Postgres silently tolerates the bad cast, which is why this stayed latent on Postgres-backed deployments.Test plan
rushx testinfoundations/server/packages/postgrespasses (83/83), including the three flipped tests and the newinferType([null])casepod-githubimage off this branch and roll it in a CockroachDB-backed deployment that has the GitHub App installed without any repositories linkedfailed to perform synclog entries for that workspaceSyncing/Wait for changescadence resumesOut of scope
storage.ts$inbuilder and theworker.tsquery shape — both are correct onceinferTypereturns a proper array typedecodeArray('{}') → [''](anotherBUG:comment in the same test file) — separate concern