Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
124886: plpgsql: improve the unsupported statement error message r=DrewKimball a=DrewKimball

This patch augments the error for attempted use of an unsupported PL/pgSQL statement with the statement tag, so that it's easier for the user to indentify the unsupported statement.

Fixes cockroachdb#123672

Release note: None

Co-authored-by: Drew Kimball <[email protected]>
  • Loading branch information
craig[bot] and DrewKimball committed May 31, 2024
2 parents 2bf8e36 + 2c67c77 commit 64bd4bb
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 12 deletions.
44 changes: 44 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/plpgsql_unsupported
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,47 @@ CREATE OR REPLACE PROCEDURE foo() AS $$
RAISE NOTICE 'x: %', x;
END
$$ LANGUAGE PLpgSQL;

subtest error_detail

# Regression test for #123672 - annotate "unsupported" errors with the
# unsupported statement type.
statement ok
CREATE TABLE mytable (inserted_by TEXT, inserted TIMESTAMP);
CREATE TABLE c (checked_user TEXT, checked_date TIMESTAMP);

statement error pgcode 0A000 DETAIL: stmt_dyn_exec is not yet supported
CREATE PROCEDURE test(checked_user TEXT, checked_date TIMESTAMP)
AS $$
DECLARE
c INT;
BEGIN
EXECUTE 'SELECT count(*) FROM mytable WHERE inserted_by = $1 AND inserted <= $2'
INTO c
USING checked_user, checked_date;
END;
$$ LANGUAGE plpgsql;

statement ok
CREATE TABLE t6 (a int);

statement error pgcode 0A000 DETAIL: stmt_case is not yet supported
CREATE PROCEDURE p6(IN i int)
LANGUAGE plpgsql
AS $$
BEGIN
INSERT INTO t6 VALUES (i);
CASE i
WHEN 6 THEN
COMMIT;
WHEN 7 THEN
ROLLBACK;
WHEN 8 THEN
COMMIT;
ELSE
ROLLBACK;
END CASE;
END;
$$;

subtest end
4 changes: 3 additions & 1 deletion pkg/sql/opt/optbuilder/plpgsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,9 @@ func (b *plpgsqlBuilder) buildPLpgSQLStatements(stmts []ast.Statement, s *scope)
return b.callContinuation(&callCon, s)

default:
panic(unsupportedPLStmtErr)
panic(errors.WithDetailf(unsupportedPLStmtErr,
"%s is not yet supported", stmt.PlpgSQLStatementTag(),
))
}
}
// Call the parent continuation to execute the rest of the function. Ignore
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/sem/plpgsqltree/statements.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ type Statement interface {
GetStmtID() uint
plpgsqlStmt()
WalkStmt(StatementVisitor) Statement
}

type TaggedStatement interface {
PlpgSQLStatementTag() string
}

Expand Down
1 change: 0 additions & 1 deletion pkg/sql/sem/plpgsqltree/utils/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,5 @@ go_library(
"//pkg/sql/sem/tree",
"//pkg/sql/sqltelemetry",
"//pkg/util/errorutil/unimplemented",
"@com_github_cockroachdb_errors//:errors",
],
)
9 changes: 2 additions & 7 deletions pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
unimp "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/errors"
)

// PLpgSQLStmtCounter is used to accurately report telemetry for plpgsql
Expand Down Expand Up @@ -61,15 +60,11 @@ var _ plpgsqltree.StatementVisitor = &telemetryVisitor{}
func (v *telemetryVisitor) Visit(
stmt plpgsqltree.Statement,
) (newStmt plpgsqltree.Statement, recurse bool) {
taggedStmt, ok := stmt.(plpgsqltree.TaggedStatement)
if !ok {
v.Err = errors.AssertionFailedf("no tag found for stmt %q", stmt)
}
tag := taggedStmt.PlpgSQLStatementTag()
tag := stmt.PlpgSQLStatementTag()
sqltelemetry.IncrementPlpgsqlStmtCounter(tag)

//Capturing telemetry for tests
_, ok = v.StmtCnt[tag]
_, ok := v.StmtCnt[tag]
if !ok {
v.StmtCnt[tag] = 1
} else {
Expand Down

0 comments on commit 64bd4bb

Please sign in to comment.