Skip to content

Commit 7dc03c6

Browse files
Nguyễn Phúc Lươngclaude
andcommitted
fix(compiler): preserve nullability when casting JSON arrow expressions (#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>
1 parent a3b0cfd commit 7dc03c6

9 files changed

Lines changed: 167 additions & 0 deletions

File tree

internal/compiler/output_columns.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,18 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er
366366
col.NotNull = false
367367
}
368368
}
369+
// A type cast does not make its argument non-nullable. If the
370+
// wrapped expression is itself nullable, the result of the cast
371+
// is too. Today we cover the common case of JSON accessor
372+
// operators (`->`, `->>`, `#>`, `#>>`) which return NULL when
373+
// the requested key/path is missing — wrapping `(data ->> 'k')`
374+
// in `::text` was previously emitting a non-nullable string.
375+
// See issue #3792.
376+
if argExpr, ok := n.Arg.(*ast.A_Expr); ok {
377+
if lang.IsJSONNullableOperator(astutils.Join(argExpr.Name, "")) {
378+
col.NotNull = false
379+
}
380+
}
369381
cols = append(cols, col)
370382

371383
case *ast.SelectStmt:

internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/db.go

Lines changed: 32 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/models.go

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/query.sql.go

Lines changed: 55 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
-- name: ListJobs :many
2+
-- A `data ->> 'key'` lookup can return SQL NULL when the key is missing.
3+
-- A surrounding `::text` cast must preserve that nullability instead of
4+
-- emitting a non-nullable Go string. See issue #3792.
5+
SELECT id,
6+
(data ->> 'PhoneNumber')::text AS phone_number,
7+
(data ->> 'ContactName')::text AS contact_name,
8+
(data ->> 'State')::text AS state
9+
FROM jobs;
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
CREATE TABLE jobs (
2+
id BIGSERIAL PRIMARY KEY,
3+
data JSONB NOT NULL
4+
);
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"version": "1",
3+
"packages": [
4+
{
5+
"path": "go",
6+
"engine": "postgresql",
7+
"sql_package": "pgx/v5",
8+
"name": "querytest",
9+
"schema": "schema.sql",
10+
"queries": "query.sql"
11+
}
12+
]
13+
}

internal/sql/lang/operator.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,18 @@ func IsMathematicalOperator(s string) bool {
4141
}
4242
return true
4343
}
44+
45+
// IsJSONNullableOperator reports whether op is a Postgres JSON / JSONB
46+
// accessor operator that returns SQL NULL when the requested key or path
47+
// is missing from the value. These are: -> (object/array element as jsonb),
48+
// ->> (object/array element as text), #> (path lookup as jsonb), and #>>
49+
// (path lookup as text). Wrapping such an expression in a type cast does
50+
// not make the result non-nullable — the key may still be absent at run
51+
// time. See issue #3792.
52+
func IsJSONNullableOperator(s string) bool {
53+
switch s {
54+
case "->", "->>", "#>", "#>>":
55+
return true
56+
}
57+
return false
58+
}

internal/sql/lang/operator_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package lang
2+
3+
import "testing"
4+
5+
func TestIsJSONNullableOperator(t *testing.T) {
6+
t.Parallel()
7+
for _, op := range []string{"->", "->>", "#>", "#>>"} {
8+
if !IsJSONNullableOperator(op) {
9+
t.Errorf("expected %q to be classified as JSON-nullable", op)
10+
}
11+
}
12+
for _, op := range []string{"", "+", "-", "=", "::", "@>", "<@", "->>>"} {
13+
if IsJSONNullableOperator(op) {
14+
t.Errorf("did not expect %q to be classified as JSON-nullable", op)
15+
}
16+
}
17+
}

0 commit comments

Comments
 (0)