fix(compiler): preserve nullability when casting JSON arrow expressions (#3792)#4458
Open
luongs3 wants to merge 1 commit into
Open
fix(compiler): preserve nullability when casting JSON arrow expressions (#3792)#4458luongs3 wants to merge 1 commit into
luongs3 wants to merge 1 commit into
Conversation
…ns (sqlc-dev#3792) PostgreSQL's JSON accessor operators (-->, ->>, #>, #>>) return SQL NULL when the requested key or path is missing. Wrapping such an expression in a type cast does not make the result non-nullable. Today sqlc emits a non-nullable Go `string` for queries like: SELECT id, (data ->> 'PhoneNumber')::text AS phone_number, (data ->> 'ContactName')::text AS contact_name FROM jobs; which crashes any consumer scanning into pgtype.Text / *string / sql.NullString when the key happens to be absent. Real Postgres returns NULL in that case; the generated row should too. Fix: * internal/sql/lang/operator.go — new IsJSONNullableOperator(s) classifier covering ->, ->>, #>, #>>. * internal/compiler/output_columns.go — in the TypeCast case, when the wrapped argument is an A_Expr whose operator IsJSONNullableOperator, set col.NotNull = false. Tight scope: only the immediate cast arg. All other cast paths (literal, column from NOT NULL source, etc.) are unchanged — confirmed via regression repros. Tests: * internal/sql/lang/operator_test.go — TestIsJSONNullableOperator positive + negative table-driven test. * internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/ — end-to-end fixture reproducing the issue under pgx/v5; golden files show pgtype.Text for the three ->>-derived columns. Verified locally: * Repro from the issue now generates pgtype.Text instead of string. * ('foo')::text, (column_not_null)::text still emit string. NULL literal cast still emits pgtype.Text (existing behavior preserved). * go test ./internal/sql/... ./internal/compiler/... ./internal/engine/... passes; gofmt -l clean on touched files; go vet clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7dc03c6 to
4aa7a49
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.
Fixes #3792.
Problem
PostgreSQL's JSON accessor operators (
->,->>,#>,#>>) return SQL NULL when the requested key or path is missing from the value. Wrapping such an expression in a type cast does not make the result non-nullable.Today sqlc emits a non-nullable Go
stringfor the issue's exact query:Generated row before this fix:
Generated row after:
Any consumer scanning into
pgtype.Text/*string/sql.NullStringpreviously crashed on rows where the JSON key happened to be absent. Real Postgres returns NULL in that case; the generated row now matches.Fix
Two small changes:
internal/sql/lang/operator.go— newIsJSONNullableOperator(s string) boolclassifier covering->,->>,#>,#>>.internal/compiler/output_columns.go— in the*ast.TypeCastcase, when the wrapped argument is an*ast.A_Exprwhose operator is JSON-nullable, setcol.NotNull = false. Tight scope: only the immediate cast argument. All other cast paths (literal cast, column-from-NOT-NULL cast, etc.) are unchanged.Broader nullability inference for casts is intentionally out of scope (the existing
// TODO Add correct, real type inferencecomment in the cast handler stays — this PR addresses the specific JSON-arrow case from #3792 without expanding into general expression-tree analysis).Tests
internal/sql/lang/operator_test.go—TestIsJSONNullableOperatortable-driven test, positive (->,->>,#>,#>>) and negative (+,-,=,@>,<@,->>>lookalike, empty).internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/— end-to-end fixture reproducing the issue's exact query under pgx/v5; golden files showpgtype.Textfor the three->>-derived columns.Verified locally
pgtype.Textinstead ofstring(the bug).('foo')::textand(name)::textwherenameis NOT NULL still emitstring— no regression on the common cast cases.(NULL)::textstill emitspgtype.Text(preserves the existing NULL-literal-cast carve-out).(data -> 'k')::jsonbcorrectly flows through the same path.go test ./internal/sql/... ./internal/compiler/... ./internal/engine/...passes;gofmt -landgo vetclean on touched files.