diff --git a/internal/compiler/output_columns.go b/internal/compiler/output_columns.go index dbd486359a..d76bf501c3 100644 --- a/internal/compiler/output_columns.go +++ b/internal/compiler/output_columns.go @@ -366,6 +366,18 @@ func (c *Compiler) outputColumns(qc *QueryCatalog, node ast.Node) ([]*Column, er col.NotNull = false } } + // A type cast does not make its argument non-nullable. If the + // wrapped expression is itself nullable, the result of the cast + // is too. Today we cover the common case of JSON accessor + // operators (`->`, `->>`, `#>`, `#>>`) which return NULL when + // the requested key/path is missing — wrapping `(data ->> 'k')` + // in `::text` was previously emitting a non-nullable string. + // See issue #3792. + if argExpr, ok := n.Arg.(*ast.A_Expr); ok { + if lang.IsJSONNullableOperator(astutils.Join(argExpr.Name, "")) { + col.NotNull = false + } + } cols = append(cols, col) case *ast.SelectStmt: diff --git a/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/db.go b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/db.go new file mode 100644 index 0000000000..0057c62319 --- /dev/null +++ b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/db.go @@ -0,0 +1,32 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 + +package querytest + +import ( + "context" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" +) + +type DBTX interface { + Exec(context.Context, string, ...interface{}) (pgconn.CommandTag, error) + Query(context.Context, string, ...interface{}) (pgx.Rows, error) + QueryRow(context.Context, string, ...interface{}) pgx.Row +} + +func New(db DBTX) *Queries { + return &Queries{db: db} +} + +type Queries struct { + db DBTX +} + +func (q *Queries) WithTx(tx pgx.Tx) *Queries { + return &Queries{ + db: tx, + } +} diff --git a/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/models.go b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/models.go new file mode 100644 index 0000000000..ed9f0ed1fe --- /dev/null +++ b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/models.go @@ -0,0 +1,10 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 + +package querytest + +type Job struct { + ID int64 + Data []byte +} diff --git a/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/query.sql.go b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/query.sql.go new file mode 100644 index 0000000000..71a052b02f --- /dev/null +++ b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/go/query.sql.go @@ -0,0 +1,55 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: query.sql + +package querytest + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const listJobs = `-- name: ListJobs :many +SELECT id, + (data ->> 'PhoneNumber')::text AS phone_number, + (data ->> 'ContactName')::text AS contact_name, + (data ->> 'State')::text AS state +FROM jobs +` + +type ListJobsRow struct { + ID int64 + PhoneNumber pgtype.Text + ContactName pgtype.Text + State pgtype.Text +} + +// A `data ->> 'key'` lookup can return SQL NULL when the key is missing. +// A surrounding `::text` cast must preserve that nullability instead of +// emitting a non-nullable Go string. See issue #3792. +func (q *Queries) ListJobs(ctx context.Context) ([]ListJobsRow, error) { + rows, err := q.db.Query(ctx, listJobs) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListJobsRow + for rows.Next() { + var i ListJobsRow + if err := rows.Scan( + &i.ID, + &i.PhoneNumber, + &i.ContactName, + &i.State, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/query.sql b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/query.sql new file mode 100644 index 0000000000..b4d80daa3c --- /dev/null +++ b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/query.sql @@ -0,0 +1,9 @@ +-- name: ListJobs :many +-- A `data ->> 'key'` lookup can return SQL NULL when the key is missing. +-- A surrounding `::text` cast must preserve that nullability instead of +-- emitting a non-nullable Go string. See issue #3792. +SELECT id, + (data ->> 'PhoneNumber')::text AS phone_number, + (data ->> 'ContactName')::text AS contact_name, + (data ->> 'State')::text AS state +FROM jobs; diff --git a/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/schema.sql b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/schema.sql new file mode 100644 index 0000000000..11ba39e32f --- /dev/null +++ b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/schema.sql @@ -0,0 +1,4 @@ +CREATE TABLE jobs ( + id BIGSERIAL PRIMARY KEY, + data JSONB NOT NULL +); diff --git a/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/sqlc.json b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/sqlc.json new file mode 100644 index 0000000000..32ede07158 --- /dev/null +++ b/internal/endtoend/testdata/json_arrow_cast_3792/postgresql/pgx/v5/sqlc.json @@ -0,0 +1,13 @@ +{ + "version": "1", + "packages": [ + { + "path": "go", + "engine": "postgresql", + "sql_package": "pgx/v5", + "name": "querytest", + "schema": "schema.sql", + "queries": "query.sql" + } + ] +} diff --git a/internal/engine/postgresql/reserved.go b/internal/engine/postgresql/reserved.go index b03a6a7e9f..191c994168 100644 --- a/internal/engine/postgresql/reserved.go +++ b/internal/engine/postgresql/reserved.go @@ -71,9 +71,13 @@ func (p *Parser) NamedParam(name string) string { } // Cast returns a type cast expression. -// PostgreSQL uses expr::type syntax. +// PostgreSQL uses expr::type syntax. Wrap the arg in parens so that +// operator precedence is preserved when the arg is a compound expression +// like a -> b or a ->> b. Parens around a simple expression are a no-op +// to the parser (pg_query normalises them away during fingerprinting), +// so this is safe for the existing simple-arg cases too. func (p *Parser) Cast(arg, typeName string) string { - return arg + "::" + typeName + return "(" + arg + ")::" + typeName } // https://www.postgresql.org/docs/current/sql-keywords-appendix.html diff --git a/internal/sql/lang/operator.go b/internal/sql/lang/operator.go index cd5ef50e38..c2eb0434e6 100644 --- a/internal/sql/lang/operator.go +++ b/internal/sql/lang/operator.go @@ -41,3 +41,18 @@ func IsMathematicalOperator(s string) bool { } return true } + +// IsJSONNullableOperator reports whether op is a Postgres JSON / JSONB +// accessor operator that returns SQL NULL when the requested key or path +// is missing from the value. These are: -> (object/array element as jsonb), +// ->> (object/array element as text), #> (path lookup as jsonb), and #>> +// (path lookup as text). Wrapping such an expression in a type cast does +// not make the result non-nullable — the key may still be absent at run +// time. See issue #3792. +func IsJSONNullableOperator(s string) bool { + switch s { + case "->", "->>", "#>", "#>>": + return true + } + return false +} diff --git a/internal/sql/lang/operator_test.go b/internal/sql/lang/operator_test.go new file mode 100644 index 0000000000..f4cee43ff6 --- /dev/null +++ b/internal/sql/lang/operator_test.go @@ -0,0 +1,17 @@ +package lang + +import "testing" + +func TestIsJSONNullableOperator(t *testing.T) { + t.Parallel() + for _, op := range []string{"->", "->>", "#>", "#>>"} { + if !IsJSONNullableOperator(op) { + t.Errorf("expected %q to be classified as JSON-nullable", op) + } + } + for _, op := range []string{"", "+", "-", "=", "::", "@>", "<@", "->>>"} { + if IsJSONNullableOperator(op) { + t.Errorf("did not expect %q to be classified as JSON-nullable", op) + } + } +}