Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
134614: sql: add hints to commonly-encountered errors r=DrewKimball a=DrewKimball

#### sql: add hint for unqualified column reference in trigger WHEN clause

This commit adds a hint to the error when a user attempts to reference
a column in a trigger's WHEN clause without qualifying it by `NEW` or
`OLD`.

Informs cockroachdb#121513

Release note: None

#### sql: add hint to error for missing parens in tuple element access

In Postgres, it is possible to reference an element of a composite-typed
column or variable like so: `colname.fieldname`. Currently, we require that
the column/variable name be wrapped in parentheses: `(colname).fieldname`.
This is often confusing for users. This commit adds a hint to the error
to make it more clear that parentheses are necessary.

Informs cockroachdb#114687
Informs cockroachdb#121513

Release note: None

Co-authored-by: Drew Kimball <[email protected]>
  • Loading branch information
craig[bot] and DrewKimball committed Nov 8, 2024
2 parents a44a9b1 + 8cffdbe commit 43b3323
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
15 changes: 15 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/plpgsql_unsupported
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
statement ok
CREATE TABLE xy (x INT, y INT);

statement error pq: unimplemented: RECORD type for PL/pgSQL variables is not yet supported.*
CREATE OR REPLACE PROCEDURE foo() AS $$
DECLARE
Expand Down Expand Up @@ -49,4 +52,16 @@ BEGIN
END;
$$;

# Add a helpful hint to the error when the user attempts to access an element of
# a composite-typed variable without parentheses.
subtest element_access_parens

statement error pgcode 42P01 pq: no data source matches prefix: val in this context\nHINT: to access a field of a composite-typed column or variable, surround the column/variable name in parentheses: \(varName\).fieldName
CREATE FUNCTION f(val xy) RETURNS xy AS $$
BEGIN
val.x := val.x + 100;
RETURN val;
END
$$ LANGUAGE PLpgSQL;

subtest end
2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/testdata/logic_test/triggers
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ statement error pgcode 42804 pq: argument of WHEN must be type bool, not type in
CREATE TRIGGER foo AFTER INSERT ON xy WHEN (1) EXECUTE FUNCTION f();

# The WHEN clause cannot reference table columns.
statement error pgcode 42703 pq: column "x" does not exist
statement error pgcode 42703 pq: column "x" does not exist\nHINT: column references in a trigger WHEN clause must be prefixed with NEW or OLD
CREATE TRIGGER foo AFTER INSERT ON xy WHEN (x = 1) EXECUTE FUNCTION f();

# The WHEN clause cannot contain a subquery.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/optbuilder/create_trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (b *Builder) buildWhenForTrigger(
// Check that the expression is of type bool. Disallow subqueries inside the
// WHEN clause.
defer b.semaCtx.Properties.Restore(b.semaCtx.Properties)
b.semaCtx.Properties.Require("WHEN", tree.RejectSubqueries)
b.semaCtx.Properties.Require(exprKindWhen.String(), tree.RejectSubqueries)
typedWhen := whenScope.resolveAndRequireType(ct.When, types.Bool)

// Check for invalid NEW or OLD variable references. Also resolve
Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/opt/optbuilder/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"fmt"
"strings"

"github.com/cockroachdb/cockroach/pkg/build"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
"github.com/cockroachdb/cockroach/pkg/sql/opt"
"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
Expand Down Expand Up @@ -1067,13 +1068,31 @@ func (s *scope) VisitPre(expr tree.Expr) (recurse bool, newExpr tree.Expr) {
// It may be a reference to a table, e.g. SELECT tbl FROM tbl.
// Attempt to resolve as a TupleStar.
if sqlerrors.IsUndefinedColumnError(resolveErr) {
if s.context == exprKindWhen {
panic(errors.WithHint(resolveErr,
"column references in a trigger WHEN clause must be prefixed with NEW or OLD"))
}
// Attempt to resolve as columnname.*, which allows items
// such as SELECT row_to_json(tbl_name) FROM tbl_name to work.
return func() (bool, tree.Expr) {
defer wrapColTupleStarPanic(resolveErr)
return s.VisitPre(columnNameAsTupleStar(string(t.ColumnName)))
}()
}
if sqlerrors.IsUndefinedRelationError(resolveErr) && t.TableName.Object() != "" {
// Attempt to resolve as columnname.fieldname in order to provide a more
// helpful error message.
_, sourceResolveErr := colinfo.ResolveColumnItem(
s.builder.ctx, s, &tree.ColumnItem{ColumnName: tree.Name(t.TableName.Object())},
)
if sourceResolveErr == nil {
panic(errors.WithIssueLink(errors.WithHint(resolveErr,
"to access a field of a composite-typed column or variable, "+
"surround the column/variable name in parentheses: (varName).fieldName"),
errors.IssueLink{IssueURL: build.MakeIssueURL(114687)},
))
}
}
panic(resolveErr)
}
return false, colI.(*scopeColumn)
Expand Down

0 comments on commit 43b3323

Please sign in to comment.