From ef0bc41022d0241ea5bcaff2c046b7f798cbe212 Mon Sep 17 00:00:00 2001 From: rharding6373 Date: Fri, 4 Aug 2023 15:06:45 -0700 Subject: [PATCH] sql: fix some error codes and messages for udfs This commit fixes some incorrect error codes and error messages in UDF logic tests that were uncovered as part of auditing pg error codes against postgres. In some cases where there were discrepancies between postgres and CRDB that were due to implementation differences or improved behavior, we left a comment above the test detailing why there is a difference between postgres and CRDB behavior. Epic: None Informs: #107369 Release note: None --- pkg/sql/logictest/testdata/logic_test/udf | 3 +-- pkg/sql/logictest/testdata/logic_test/udf_insert | 5 ++--- pkg/sql/logictest/testdata/logic_test/udf_options | 6 +++--- pkg/sql/logictest/testdata/logic_test/udf_privileges | 2 +- pkg/sql/logictest/testdata/logic_test/udf_record | 11 ++++++----- .../logictest/testdata/logic_test/udf_schema_change | 2 +- pkg/sql/logictest/testdata/logic_test/udf_update | 1 - .../testdata/logic_test/udf_volatility_check | 3 ++- pkg/sql/opt/optbuilder/create_function.go | 4 ++-- pkg/sql/opt/optbuilder/insert.go | 2 +- pkg/sql/sem/tree/function_definition.go | 2 +- 11 files changed, 20 insertions(+), 21 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf index 1ab1534976f9..431c2aa29b0a 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf +++ b/pkg/sql/logictest/testdata/logic_test/udf @@ -406,8 +406,7 @@ CREATE FUNCTION sc1.f_test_drop(IN INT8) SELECT 1; $$ -# TODO(107369): Postgres errors with code 42725 here. -statement error pgcode XXUUU pq: function name \"f_test_drop\" is not unique +statement error pgcode 42725 pq: function name \"f_test_drop\" is not unique DROP FUNCTION f_test_drop; statement ok diff --git a/pkg/sql/logictest/testdata/logic_test/udf_insert b/pkg/sql/logictest/testdata/logic_test/udf_insert index c647fdbc8168..751e9f0e9b6e 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_insert +++ b/pkg/sql/logictest/testdata/logic_test/udf_insert @@ -18,9 +18,8 @@ SELECT f_void(); ---- NULL -# TODO(107369): This doesn't error in postgres, and this is the error code for -# invalid foreign key, which may not make sense. -statement error pgcode 42830 missing "a" primary key column +# Note: This does not error in postgres until the function is executed. +statement error pgcode 23502 missing "a" primary key column CREATE FUNCTION f_err(b INT) RETURNS RECORD AS $$ INSERT INTO t (b) VALUES (b); diff --git a/pkg/sql/logictest/testdata/logic_test/udf_options b/pkg/sql/logictest/testdata/logic_test/udf_options index 68409242f5c8..a95c0e427887 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_options +++ b/pkg/sql/logictest/testdata/logic_test/udf_options @@ -109,13 +109,13 @@ l1 l2 i1 i2 s1 s2 v1 v2 statement ok CREATE SEQUENCE sq2; -# TODO(107369): This does not error in postgres. +# Note: postgres allows non-volatile functions to call other volatile functions. statement error pgcode 22023 volatile statement not allowed in immutable function: SELECT nextval\('sq2'\) CREATE FUNCTION rand_i() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$SELECT nextval('sq2')$$; -# TODO(107369): This does not error in postgres. +# Note: postgres allows non-volatile functions to call other volatile functions. statement error pgcode 22023 volatile statement not allowed in stable function: SELECT nextval\('sq2'\) -CREATE FUNCTION rand_s() RETURNS INT STABLE LANGUAGE SQL AS $$SELECT nextval('sq2')$$; +CREATE FUNCTION rand_s() RETURNS INT STABLE LANGUAGE SQL AS $$SELECT nextval('sq2')$$; statement ok CREATE FUNCTION rand_v() RETURNS INT VOLATILE LANGUAGE SQL AS $$SELECT nextval('sq2')$$; diff --git a/pkg/sql/logictest/testdata/logic_test/udf_privileges b/pkg/sql/logictest/testdata/logic_test/udf_privileges index 7430b75b8407..002f1aa37feb 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_privileges +++ b/pkg/sql/logictest/testdata/logic_test/udf_privileges @@ -533,7 +533,7 @@ CREATE FUNCTION f_test_show_grants(INT, string, OID) RETURNS INT LANGUAGE SQL AS CREATE USER u_test_show_grants; GRANT EXECUTE ON FUNCTION f_test_show_grants(INT), f_test_show_grants(INT, string, OID) TO u_test_show_grants; -statement error pgcode XXUUU pq: function name "f_test_show_grants" is not unique +statement error pgcode 42725 pq: function name "f_test_show_grants" is not unique SHOW GRANTS ON FUNCTION f_test_show_grants; query TTTTTTB colnames diff --git a/pkg/sql/logictest/testdata/logic_test/udf_record b/pkg/sql/logictest/testdata/logic_test/udf_record index 915d1a30d477..036d7711bafb 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_record +++ b/pkg/sql/logictest/testdata/logic_test/udf_record @@ -535,20 +535,21 @@ SELECT imp_cast(), (1,2,'3') = imp_cast(), pg_typeof(imp_cast()) ---- (1,2,3) true imp -# TODO(107369): This is code 42846 in postgres. +# Note: postgres returns error code 42846 (cannot cast type) here instead due to +# implementation differences. statement error pgcode 42P13 return type mismatch in function declared to return imp CREATE FUNCTION err() RETURNS imp LANGUAGE SQL AS $$ SELECT (1, 2) $$ -# TODO(mgartner): The error message should say "imp" instead of "record". -statement error pgcode 42P13 return type mismatch in function declared to return record +statement error pgcode 42P13 return type mismatch in function declared to return imp CREATE FUNCTION err() RETURNS imp LANGUAGE SQL AS $$ SELECT k, a FROM imp $$ -# TODO(mgartner): The error message should say "imp" instead of "record". -statement error pgcode 42P13 return type mismatch in function declared to return record +# Note: This function can be successfully created in postgres, but will fail +# when called. +statement error pgcode 42P13 return type mismatch in function declared to return imp CREATE FUNCTION err() RETURNS imp LANGUAGE SQL AS $$ SELECT k, a, b::INT FROM imp $$ diff --git a/pkg/sql/logictest/testdata/logic_test/udf_schema_change b/pkg/sql/logictest/testdata/logic_test/udf_schema_change index 5ae7262c8ed5..bd65de82408d 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_schema_change +++ b/pkg/sql/logictest/testdata/logic_test/udf_schema_change @@ -633,7 +633,7 @@ ALTER TABLE t_alter ALTER b TYPE TEXT statement ok ALTER TABLE t_alter ADD COLUMN d INT; -statement error pgcode 42P13 pq: return type mismatch in function declared to return record +statement error pgcode 42P13 pq: return type mismatch in function declared to return t_alter CREATE OR REPLACE FUNCTION f_rtbl() RETURNS t_alter LANGUAGE SQL AS $$ SELECT 1, 'foobar', 2 $$ diff --git a/pkg/sql/logictest/testdata/logic_test/udf_update b/pkg/sql/logictest/testdata/logic_test/udf_update index 4c7d7176febb..5233745b1a87 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_update +++ b/pkg/sql/logictest/testdata/logic_test/udf_update @@ -167,7 +167,6 @@ CREATE TABLE generated_as_id_t ( statement ok INSERT INTO generated_as_id_t (a) VALUES (7), (8), (9); -# TODO(107369): This error code does not have a name associated with it. statement error pgcode 428C9 column "b" can only be updated to DEFAULT CREATE FUNCTION f_err(i INT, j INT) RETURNS RECORD AS $$ diff --git a/pkg/sql/logictest/testdata/logic_test/udf_volatility_check b/pkg/sql/logictest/testdata/logic_test/udf_volatility_check index a3bce3258887..68721d0d2af4 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_volatility_check +++ b/pkg/sql/logictest/testdata/logic_test/udf_volatility_check @@ -5,7 +5,8 @@ statement ok CREATE TABLE t1(a INT PRIMARY KEY, b INT); CREATE TABLE t2(a INT PRIMARY KEY, b INT); -# TODO(107369): These do not appear to error in postgres. +# Note: postgres does not error in the following cases. CRDB provides stronger +# protections against adding volatility to non-volatile functions. statement error pgcode 22023 pq: referencing relations is not allowed in immutable function CREATE FUNCTION f() RETURNS FLOAT LANGUAGE SQL IMMUTABLE AS $$ SELECT a FROM t1 $$; diff --git a/pkg/sql/opt/optbuilder/create_function.go b/pkg/sql/opt/optbuilder/create_function.go index ee9550642f0e..72e9744abce5 100644 --- a/pkg/sql/opt/optbuilder/create_function.go +++ b/pkg/sql/opt/optbuilder/create_function.go @@ -358,7 +358,7 @@ func validateReturnType(expected *types.T, cols []scopeColumn) error { if !typ.Equivalent(cols[i].typ) { return pgerror.WithCandidateCode( errors.WithDetailf( - errors.Newf("return type mismatch in function declared to return record"), + errors.Newf("return type mismatch in function declared to return %s", expected.Name()), "Final statement returns %s instead of %s at column %d", cols[i].typ.Name(), typ.Name(), i+1, ), @@ -372,7 +372,7 @@ func validateReturnType(expected *types.T, cols []scopeColumn) error { // Ran out of columns from last statement. return pgerror.WithCandidateCode( errors.WithDetailf( - errors.New("return type mismatch in function declared to return record"), + errors.Newf("return type mismatch in function declared to return %s", expected.Name()), "Final statement returns too few columns", ), pgcode.InvalidFunctionDefinition, diff --git a/pkg/sql/opt/optbuilder/insert.go b/pkg/sql/opt/optbuilder/insert.go index 6293404fd714..0385d59c2282 100644 --- a/pkg/sql/opt/optbuilder/insert.go +++ b/pkg/sql/opt/optbuilder/insert.go @@ -454,7 +454,7 @@ func (mb *mutationBuilder) checkPrimaryKeyForInsert() { continue } - panic(pgerror.Newf(pgcode.InvalidForeignKey, + panic(pgerror.Newf(pgcode.NotNullViolation, "missing %q primary key column", col.ColName())) } } diff --git a/pkg/sql/sem/tree/function_definition.go b/pkg/sql/sem/tree/function_definition.go index 712ffdb1f535..9545d45dfe7b 100644 --- a/pkg/sql/sem/tree/function_definition.go +++ b/pkg/sql/sem/tree/function_definition.go @@ -298,7 +298,7 @@ func (fd *ResolvedFunctionDefinition) MatchOverload( ) } if len(ret) > 1 { - return QualifiedOverload{}, errors.Errorf("function name %q is not unique", fd.Name) + return QualifiedOverload{}, pgerror.Newf(pgcode.AmbiguousFunction, "function name %q is not unique", fd.Name) } return ret[0], nil }