From 6c604446e7459008789769c1bc181c719c9ef746 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Fri, 6 Oct 2023 19:27:33 -0400 Subject: [PATCH] sql,clusterversion: add a cluster version for procedures Release note: None --- .../settings/settings-for-tenants.txt | 2 +- docs/generated/settings/settings.html | 2 +- pkg/clusterversion/cockroach_versions.go | 8 +- .../testdata/logic_test/crdb_internal | 149 ++++++++++-------- .../testdata/logic_test/drop_procedure | 2 + .../logic_test/mixed_version_procedure | 49 ++++++ .../logictest/testdata/logic_test/procedure | 2 + .../testdata/logic_test/procedure_plpgsql | 2 + .../testdata/logic_test/procedure_privileges | 2 + .../logictest/testdata/logic_test/show_create | 6 + .../testdata/logic_test/udf_privileges | 7 +- .../BUILD.bazel | 2 +- .../generated_test.go | 7 + .../local-mixed-22.2-23.1/generated_test.go | 28 ---- pkg/sql/opt/optbuilder/BUILD.bazel | 1 + pkg/sql/opt/optbuilder/create_function.go | 6 + 16 files changed, 174 insertions(+), 101 deletions(-) create mode 100644 pkg/sql/logictest/testdata/logic_test/mixed_version_procedure diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index f0b58d5a6d0a..76dd75ab1ad2 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -317,4 +317,4 @@ trace.snapshot.rate duration 0s if non-zero, interval at which background trace trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https:///#/debug/tracez application trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. application ui.display_timezone enumeration etc/utc the timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1] application -version version 1000023.1-32 set the active cluster version in the format '.' application +version version 1000023.1-34 set the active cluster version in the format '.' application diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index c60bf00fe17f..6582b4b5c64e 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -269,6 +269,6 @@
trace.span_registry.enabled
booleantrueif set, ongoing traces can be seen at https://<ui>/#/debug/tracezServerless/Dedicated/Self-Hosted
trace.zipkin.collector
stringthe address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.Serverless/Dedicated/Self-Hosted
ui.display_timezone
enumerationetc/utcthe timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]Serverless/Dedicated/Self-Hosted -
version
version1000023.1-32set the active cluster version in the format '<major>.<minor>'Serverless/Dedicated/Self-Hosted +
version
version1000023.1-34set the active cluster version in the format '<major>.<minor>'Serverless/Dedicated/Self-Hosted diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 44c23a5b7d6b..de4adca705bb 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -514,6 +514,9 @@ const ( // {statement|transaction}_execution_insights system tables. V23_2_AddSystemExecInsightsTable + // V23_2_Procedures is the version where procedures are enabled. + V23_2_Procedures + // ************************************************* // Step (1) Add new versions here. // Do not add new versions to a patch release. @@ -855,11 +858,14 @@ var rawVersionsSingleton = keyedVersions{ Key: V23_2_MVCCStatisticsTable, Version: roachpb.Version{Major: 23, Minor: 1, Internal: 30}, }, - { Key: V23_2_AddSystemExecInsightsTable, Version: roachpb.Version{Major: 23, Minor: 1, Internal: 32}, }, + { + Key: V23_2_Procedures, + Version: roachpb.Version{Major: 23, Minor: 1, Internal: 34}, + }, // ************************************************* // Step (2): Add new versions here. diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index a6951306cc76..9caec2088c56 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -1458,6 +1458,7 @@ CREATE FUNCTION f(STRING, b INT) RETURNS STRING STRICT IMMUTABLE LEAKPROOF LANGU # Add a procedure to ensure that it doesn't show up in the # create_function_statements table. +skipif config local-mixed-22.2-23.1 statement ok CREATE PROCEDURE f(BOOL) LANGUAGE SQL AS $$ SELECT 1 $$; @@ -1467,39 +1468,39 @@ CREATE SCHEMA sc; statement CREATE FUNCTION sc.f2(STRING) RETURNS STRING LANGUAGE SQL AS $$ SELECT 'hello' $$; -query ITITITT -SELECT database_id, database_name, schema_id, schema_name, function_id, function_name, create_statement +query TTTT +SELECT database_name, schema_name, function_name, create_statement FROM crdb_internal.create_function_statements WHERE function_name IN ('f', 'f2') ORDER BY function_id; ---- -104 test 105 public 129 f CREATE FUNCTION public.f(IN INT8) - RETURNS INT8 - VOLATILE - NOT LEAKPROOF - CALLED ON NULL INPUT - LANGUAGE SQL - AS $$ - SELECT 1; - $$ -104 test 105 public 130 f CREATE FUNCTION public.f(IN STRING, IN b INT8) - RETURNS STRING - IMMUTABLE - LEAKPROOF - STRICT - LANGUAGE SQL - AS $$ - SELECT 'hello'; - $$ -104 test 132 sc 133 f2 CREATE FUNCTION sc.f2(IN STRING) - RETURNS STRING - VOLATILE - NOT LEAKPROOF - CALLED ON NULL INPUT - LANGUAGE SQL - AS $$ - SELECT 'hello'; - $$ +test public f CREATE FUNCTION public.f(IN INT8) + RETURNS INT8 + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE SQL + AS $$ + SELECT 1; + $$ +test public f CREATE FUNCTION public.f(IN STRING, IN b INT8) + RETURNS STRING + IMMUTABLE + LEAKPROOF + STRICT + LANGUAGE SQL + AS $$ + SELECT 'hello'; + $$ +test sc f2 CREATE FUNCTION sc.f2(IN STRING) + RETURNS STRING + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE SQL + AS $$ + SELECT 'hello'; + $$ statement ok CREATE DATABASE test_cross_db; @@ -1507,53 +1508,56 @@ USE test_cross_db; CREATE FUNCTION f_cross_db() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$; USE test; -query ITITITT -SELECT database_id, database_name, schema_id, schema_name, function_id, function_name, create_statement +query TTTT +SELECT database_name, schema_name, function_name, create_statement FROM "".crdb_internal.create_function_statements WHERE function_name IN ('f', 'f2', 'f_cross_db') ORDER BY function_id; ---- -104 test 105 public 129 f CREATE FUNCTION public.f(IN INT8) - RETURNS INT8 - VOLATILE - NOT LEAKPROOF - CALLED ON NULL INPUT - LANGUAGE SQL - AS $$ - SELECT 1; - $$ -104 test 105 public 130 f CREATE FUNCTION public.f(IN STRING, IN b INT8) - RETURNS STRING - IMMUTABLE - LEAKPROOF - STRICT - LANGUAGE SQL - AS $$ - SELECT 'hello'; - $$ -104 test 132 sc 133 f2 CREATE FUNCTION sc.f2(IN STRING) - RETURNS STRING - VOLATILE - NOT LEAKPROOF - CALLED ON NULL INPUT - LANGUAGE SQL - AS $$ - SELECT 'hello'; - $$ -134 test_cross_db 135 public 136 f_cross_db CREATE FUNCTION public.f_cross_db() - RETURNS INT8 - VOLATILE - NOT LEAKPROOF - CALLED ON NULL INPUT - LANGUAGE SQL - AS $$ - SELECT 1; - $$ +test public f CREATE FUNCTION public.f(IN INT8) + RETURNS INT8 + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE SQL + AS $$ + SELECT 1; + $$ +test public f CREATE FUNCTION public.f(IN STRING, IN b INT8) + RETURNS STRING + IMMUTABLE + LEAKPROOF + STRICT + LANGUAGE SQL + AS $$ + SELECT 'hello'; + $$ +test sc f2 CREATE FUNCTION sc.f2(IN STRING) + RETURNS STRING + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE SQL + AS $$ + SELECT 'hello'; + $$ +test_cross_db public f_cross_db CREATE FUNCTION public.f_cross_db() + RETURNS INT8 + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE SQL + AS $$ + SELECT 1; + $$ + +skipif config local-mixed-22.2-23.1 +statement ok +DROP PROCEDURE f(BOOL); statement ok DROP FUNCTION f(INT); DROP FUNCTION f(STRING, INT); -DROP PROCEDURE f(BOOL); DROP FUNCTION sc.f2(STRING); DROP SCHEMA sc; USE test_cross_db; @@ -1566,23 +1570,29 @@ subtest end subtest create_procedure_statements +skipif config local-mixed-22.2-23.1 statement ok CREATE PROCEDURE p(INT) LANGUAGE SQL AS $$ SELECT 1 $$; +skipif config local-mixed-22.2-23.1 statement ok CREATE PROCEDURE p(STRING, b INT) LANGUAGE SQL AS $$ SELECT 'hello' $$; # Add a function to ensure that it doesn't show up in the # create_procedure_statements table. +skipif config local-mixed-22.2-23.1 statement ok CREATE FUNCTION p(BOOL) RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$; +skipif config local-mixed-22.2-23.1 statement ok CREATE SCHEMA sc; +skipif config local-mixed-22.2-23.1 statement CREATE PROCEDURE sc.p2(STRING) LANGUAGE SQL AS $$ SELECT 'hello' $$; +skipif config local-mixed-22.2-23.1 query ITITITT SELECT database_id, database_name, schema_id, schema_name, procedure_id, procedure_name, create_statement FROM crdb_internal.create_procedure_statements @@ -1605,12 +1615,14 @@ ORDER BY procedure_id; SELECT 'hello'; $$ +skipif config local-mixed-22.2-23.1 statement ok CREATE DATABASE test_cross_db; USE test_cross_db; CREATE PROCEDURE p_cross_db() LANGUAGE SQL AS $$ SELECT 1 $$; USE test; +skipif config local-mixed-22.2-23.1 query ITITITT SELECT database_id, database_name, schema_id, schema_name, procedure_id, procedure_name, create_statement FROM "".crdb_internal.create_procedure_statements @@ -1638,6 +1650,7 @@ ORDER BY procedure_id; SELECT 1; $$ +skipif config local-mixed-22.2-23.1 statement ok DROP PROCEDURE p(INT); DROP PROCEDURE p(STRING, INT); diff --git a/pkg/sql/logictest/testdata/logic_test/drop_procedure b/pkg/sql/logictest/testdata/logic_test/drop_procedure index b2b1bec73bef..af37a520d1af 100644 --- a/pkg/sql/logictest/testdata/logic_test/drop_procedure +++ b/pkg/sql/logictest/testdata/logic_test/drop_procedure @@ -1,3 +1,5 @@ +# LogicTest: !local-mixed-22.2-23.1 + statement ok CREATE PROCEDURE p_test_drop() LANGUAGE SQL AS 'SELECT 1' diff --git a/pkg/sql/logictest/testdata/logic_test/mixed_version_procedure b/pkg/sql/logictest/testdata/logic_test/mixed_version_procedure new file mode 100644 index 000000000000..4ff792989a3b --- /dev/null +++ b/pkg/sql/logictest/testdata/logic_test/mixed_version_procedure @@ -0,0 +1,49 @@ +# LogicTest: cockroach-go-testserver-upgrade-to-master + +# Verify that all nodes are running the previous version. + +query T nodeidx=0 +SELECT crdb_internal.node_executable_version() +---- +23.1 + +query T nodeidx=1 +SELECT crdb_internal.node_executable_version() +---- +23.1 + +query T nodeidx=2 +SELECT crdb_internal.node_executable_version() +---- +23.1 + +upgrade 0 + +user root nodeidx=0 + +# Creating a procedure should fail with an unimplemented error. +statement error unimplemented: procedures are not yet supported +CREATE PROCEDURE p() LANGUAGE SQL AS 'SELECT 1' + +user root nodeidx=1 + +# These statements should fail with a parsing error. +statement error at or near "procedure": syntax error +CREATE PROCEDURE p() LANGUAGE SQL AS 'SELECT 1' + +# Upgrade all nodes. + +upgrade 1 + +upgrade 2 + +# Makes sure the upgrade job has finished, and the cluster version gate is +# passed. +query B retry +SELECT crdb_internal.is_at_least_version('23.1-32') +---- +true + +# Creating a procedure should now be possible. +statement ok +CREATE PROCEDURE p() LANGUAGE SQL AS 'SELECT 1' diff --git a/pkg/sql/logictest/testdata/logic_test/procedure b/pkg/sql/logictest/testdata/logic_test/procedure index 54ec463d5517..712f465e6242 100644 --- a/pkg/sql/logictest/testdata/logic_test/procedure +++ b/pkg/sql/logictest/testdata/logic_test/procedure @@ -1,3 +1,5 @@ +# LogicTest: !local-mixed-22.2-23.1 + statement ok CREATE PROCEDURE p() LANGUAGE SQL AS 'SELECT 1' diff --git a/pkg/sql/logictest/testdata/logic_test/procedure_plpgsql b/pkg/sql/logictest/testdata/logic_test/procedure_plpgsql index 766d03a99d79..6173f5d3406d 100644 --- a/pkg/sql/logictest/testdata/logic_test/procedure_plpgsql +++ b/pkg/sql/logictest/testdata/logic_test/procedure_plpgsql @@ -1,3 +1,5 @@ +# LogicTest: !local-mixed-22.2-23.1 + statement ok CREATE TABLE t ( k INT PRIMARY KEY, diff --git a/pkg/sql/logictest/testdata/logic_test/procedure_privileges b/pkg/sql/logictest/testdata/logic_test/procedure_privileges index 9421584d6e3c..91f68511d102 100644 --- a/pkg/sql/logictest/testdata/logic_test/procedure_privileges +++ b/pkg/sql/logictest/testdata/logic_test/procedure_privileges @@ -1,3 +1,5 @@ +# LogicTest: !local-mixed-22.2-23.1 + subtest grant_revoke statement ok diff --git a/pkg/sql/logictest/testdata/logic_test/show_create b/pkg/sql/logictest/testdata/logic_test/show_create index 4d36bafa5193..fdb2e334dc1d 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_create +++ b/pkg/sql/logictest/testdata/logic_test/show_create @@ -205,9 +205,11 @@ CREATE FUNCTION r1() RETURNS INT LANGUAGE SQL AS 'SELECT 1' statement ok CREATE FUNCTION r1(i INT) RETURNS INT LANGUAGE SQL AS 'SELECT 1' +skipif config local-mixed-22.2-23.1 statement ok CREATE PROCEDURE r1(s STRING) LANGUAGE SQL AS 'SELECT 1' +skipif config local-mixed-22.2-23.1 statement ok CREATE PROCEDURE r1(s STRING, i INT) LANGUAGE SQL AS 'SELECT 1' @@ -233,6 +235,7 @@ r1 CREATE FUNCTION public.r1(IN i INT8) SELECT 1; $$ +skipif config local-mixed-22.2-23.1 query TT SELECT * FROM [SHOW CREATE PROCEDURE r1] ORDER BY 2 ---- @@ -253,12 +256,14 @@ CREATE SCHEMA sc statement ok CREATE FUNCTION sc.r2() RETURNS INT LANGUAGE SQL AS 'SELECT 1' +skipif config local-mixed-22.2-23.1 statement ok CREATE PROCEDURE sc.r2(s STRING) LANGUAGE SQL AS 'SELECT 1' statement error pgcode 42883 pq: unknown function: r2() SHOW CREATE FUNCTION r2; +skipif config local-mixed-22.2-23.1 statement error pgcode 42883 pq: unknown procedure: r2() SHOW CREATE PROCEDURE r2; @@ -278,6 +283,7 @@ r2 CREATE FUNCTION sc.r2() SELECT 1; $$ +skipif config local-mixed-22.2-23.1 query TT SHOW CREATE PROCEDURE r2 ---- diff --git a/pkg/sql/logictest/testdata/logic_test/udf_privileges b/pkg/sql/logictest/testdata/logic_test/udf_privileges index 20292d54901c..ce8a9b5b9726 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_privileges +++ b/pkg/sql/logictest/testdata/logic_test/udf_privileges @@ -583,8 +583,13 @@ CREATE SCHEMA sc_test_show_grants; SET search_path = sc_test_show_grants; CREATE FUNCTION f_test_show_grants(INT) RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$; CREATE FUNCTION f_test_show_grants(INT, string, OID) RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$; -CREATE PROCEDURE test_priv_p() LANGUAGE SQL AS $$ SELECT 1 $$; CREATE USER u_test_show_grants; + +skipif config local-mixed-22.2-23.1 +statement ok +CREATE PROCEDURE test_priv_p() LANGUAGE SQL AS $$ SELECT 1 $$; + +statement ok GRANT EXECUTE ON FUNCTION f_test_show_grants(INT), f_test_show_grants(INT, string, OID) TO u_test_show_grants; statement error pgcode 42725 pq: function name "f_test_show_grants" is not unique diff --git a/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/BUILD.bazel b/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/BUILD.bazel index 15cc14daa5ae..45a04f517076 100644 --- a/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/BUILD.bazel +++ b/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/BUILD.bazel @@ -15,7 +15,7 @@ go_test( "//pkg/sql/logictest:testdata", # keep ], exec_properties = {"Pool": "large"}, - shard_count = 14, + shard_count = 15, tags = [ "cpu:2", ], diff --git a/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/generated_test.go b/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/generated_test.go index 418bd7186b1d..3b0348d6f578 100644 --- a/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/generated_test.go +++ b/pkg/sql/logictest/tests/cockroach-go-testserver-upgrade-to-master/generated_test.go @@ -120,6 +120,13 @@ func TestLogic_mixed_version_partially_visible_index( runLogicTest(t, "mixed_version_partially_visible_index") } +func TestLogic_mixed_version_procedure( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "mixed_version_procedure") +} + func TestLogic_mixed_version_range_tombstones( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go b/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go index c8a5dcf729f3..0d315381e76c 100644 --- a/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go +++ b/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go @@ -652,13 +652,6 @@ func TestLogic_drop_owned_by( runLogicTest(t, "drop_owned_by") } -func TestLogic_drop_procedure( - t *testing.T, -) { - defer leaktest.AfterTest(t)() - runLogicTest(t, "drop_procedure") -} - func TestLogic_drop_role_with_default_privileges( t *testing.T, ) { @@ -1394,27 +1387,6 @@ func TestLogic_privileges_table( runLogicTest(t, "privileges_table") } -func TestLogic_procedure( - t *testing.T, -) { - defer leaktest.AfterTest(t)() - runLogicTest(t, "procedure") -} - -func TestLogic_procedure_plpgsql( - t *testing.T, -) { - defer leaktest.AfterTest(t)() - runLogicTest(t, "procedure_plpgsql") -} - -func TestLogic_procedure_privileges( - t *testing.T, -) { - defer leaktest.AfterTest(t)() - runLogicTest(t, "procedure_privileges") -} - func TestLogic_propagate_input_ordering( t *testing.T, ) { diff --git a/pkg/sql/opt/optbuilder/BUILD.bazel b/pkg/sql/opt/optbuilder/BUILD.bazel index 9acbe42ef2e3..d84931a653be 100644 --- a/pkg/sql/opt/optbuilder/BUILD.bazel +++ b/pkg/sql/opt/optbuilder/BUILD.bazel @@ -50,6 +50,7 @@ go_library( importpath = "github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder", visibility = ["//visibility:public"], deps = [ + "//pkg/clusterversion", "//pkg/kv/kvserver/concurrency/isolation", "//pkg/server/telemetry", "//pkg/settings", diff --git a/pkg/sql/opt/optbuilder/create_function.go b/pkg/sql/opt/optbuilder/create_function.go index cc7ddb2fb6cd..c5ada2248535 100644 --- a/pkg/sql/opt/optbuilder/create_function.go +++ b/pkg/sql/opt/optbuilder/create_function.go @@ -14,6 +14,7 @@ import ( "context" "fmt" + "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/funcinfo" "github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc" @@ -39,6 +40,11 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateRoutine, inScope *scope) (o } } + activeVersion := b.evalCtx.Settings.Version.ActiveVersion(b.ctx) + if cf.IsProcedure && !activeVersion.IsActive(clusterversion.V23_2_Procedures) { + panic(unimplemented.New("procedures", "procedures are not yet supported")) + } + sch, resName := b.resolveSchemaForCreateFunction(&cf.Name) schID := b.factory.Metadata().AddSchema(sch) cf.Name.ObjectNamePrefix = resName