From 2c67c777e46509db2722c23c275568fb8709eb2f Mon Sep 17 00:00:00 2001
From: Drew Kimball <drewk@cockroachlabs.com>
Date: Thu, 30 May 2024 16:14:16 -0600
Subject: [PATCH] plpgsql: improve the unsupported statement error message

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 #123672

Release note: None
---
 .../testdata/logic_test/plpgsql_unsupported   | 44 +++++++++++++++++++
 pkg/sql/opt/optbuilder/plpgsql.go             |  4 +-
 pkg/sql/sem/plpgsqltree/statements.go         |  3 --
 pkg/sql/sem/plpgsqltree/utils/BUILD.bazel     |  1 -
 pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go |  9 +---
 5 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/pkg/ccl/logictestccl/testdata/logic_test/plpgsql_unsupported b/pkg/ccl/logictestccl/testdata/logic_test/plpgsql_unsupported
index 6b855758541d..3ea7ef507436 100644
--- a/pkg/ccl/logictestccl/testdata/logic_test/plpgsql_unsupported
+++ b/pkg/ccl/logictestccl/testdata/logic_test/plpgsql_unsupported
@@ -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
diff --git a/pkg/sql/opt/optbuilder/plpgsql.go b/pkg/sql/opt/optbuilder/plpgsql.go
index 747ee6e578d1..bf3d38fba357 100644
--- a/pkg/sql/opt/optbuilder/plpgsql.go
+++ b/pkg/sql/opt/optbuilder/plpgsql.go
@@ -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
diff --git a/pkg/sql/sem/plpgsqltree/statements.go b/pkg/sql/sem/plpgsqltree/statements.go
index 071364a24120..dd1270d5b1d7 100644
--- a/pkg/sql/sem/plpgsqltree/statements.go
+++ b/pkg/sql/sem/plpgsqltree/statements.go
@@ -27,9 +27,6 @@ type Statement interface {
 	GetStmtID() uint
 	plpgsqlStmt()
 	WalkStmt(StatementVisitor) Statement
-}
-
-type TaggedStatement interface {
 	PlpgSQLStatementTag() string
 }
 
diff --git a/pkg/sql/sem/plpgsqltree/utils/BUILD.bazel b/pkg/sql/sem/plpgsqltree/utils/BUILD.bazel
index 196658f1391f..ce0a8945e1d8 100644
--- a/pkg/sql/sem/plpgsqltree/utils/BUILD.bazel
+++ b/pkg/sql/sem/plpgsqltree/utils/BUILD.bazel
@@ -11,6 +11,5 @@ go_library(
         "//pkg/sql/sem/tree",
         "//pkg/sql/sqltelemetry",
         "//pkg/util/errorutil/unimplemented",
-        "@com_github_cockroachdb_errors//:errors",
     ],
 )
diff --git a/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go b/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go
index 64835dc8b0d6..efafddf95beb 100644
--- a/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go
+++ b/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go
@@ -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
@@ -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 {