From c619b198c8411e34151bdc58d68abdcef574ec44 Mon Sep 17 00:00:00 2001 From: maryliag Date: Fri, 3 Feb 2023 13:05:15 -0500 Subject: [PATCH 01/10] ui: fix time scale selection When making a call to retrieve the list of fingerprints per index, it was suppose to use the round dates from the time picker, otherwise it wouldn't get the rounded hour that has the aggregated timestamp. Epic: None Release note: None --- pkg/ui/workspaces/cluster-ui/src/api/indexDetailsApi.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/api/indexDetailsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/indexDetailsApi.ts index 13f48db37245..5ff916c99761 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/indexDetailsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/indexDetailsApi.ts @@ -20,7 +20,7 @@ import { StatementRawFormat, } from "src/api"; import moment from "moment"; -import { TimeScale, toDateRange } from "../timeScaleDropdown"; +import { TimeScale, toRoundedDateRange } from "../timeScaleDropdown"; import { AggregateStatistics } from "../statementsTable"; import { INTERNAL_APP_NAME_PREFIX } from "../recentExecutions/recentStatementUtils"; @@ -79,7 +79,7 @@ export function StatementsListRequestFromDetails( ts: TimeScale, ): StatementsUsingIndexRequest { if (ts === null) return { table, index, database }; - const [start, end] = toDateRange(ts); + const [start, end] = toRoundedDateRange(ts); return { table, index, database, start, end }; } From b9b9ec4beefe3aa1255aad55083baf2bcd0ebc99 Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Fri, 3 Feb 2023 10:48:35 -0500 Subject: [PATCH 02/10] sql: udf volatility check This commits adds a simple udf volatility check by utilizing the volatilities collected by the optbuilder when building a statement. Errors are throw if any statement is not as strict as the target volatility. Release note (sql change): Previously, UDFs can be created with any volatility no matter if the function body statements contain any expression which would violate the target volatility. For example, an immutable function might use `random()` in it. This change added validations to guarantee that all statements in the function body should be as strict as the expected UDF volatility. --- .../tests/3node-tenant/generated_test.go | 7 ++ pkg/sql/alter_function.go | 29 +++++- pkg/sql/create_function_test.go | 6 +- pkg/sql/drop_function_test.go | 8 +- pkg/sql/function_resolver_test.go | 20 ++-- pkg/sql/logictest/testdata/logic_test/udf | 65 +++++------- .../testdata/logic_test/udf_volatility_check | 98 +++++++++++++++++++ .../tests/fakedist-disk/generated_test.go | 7 ++ .../tests/fakedist-vec-off/generated_test.go | 7 ++ .../tests/fakedist/generated_test.go | 7 ++ .../generated_test.go | 7 ++ .../tests/local-vec-off/generated_test.go | 7 ++ .../logictest/tests/local/generated_test.go | 7 ++ pkg/sql/opt/optbuilder/create_function.go | 37 ++++++- .../scbuild/testdata/create_function | 4 +- .../scbuild/testdata/drop_function | 4 +- .../schemachanger/scdecomp/testdata/function | 4 +- .../scplan/testdata/create_function | 6 +- .../scplan/testdata/drop_function | 2 +- .../testdata/end_to_end/create_function | 6 +- .../testdata/end_to_end/drop_function | 6 +- .../testdata/explain/create_function | 8 +- .../testdata/explain/drop_function | 2 +- .../testdata/explain_verbose/create_function | 8 +- .../testdata/explain_verbose/drop_function | 2 +- pkg/sql/sem/tree/udf.go | 13 +++ 26 files changed, 290 insertions(+), 87 deletions(-) create mode 100644 pkg/sql/logictest/testdata/logic_test/udf_volatility_check diff --git a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go index fa92cd05e1a8..888c50768412 100644 --- a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go +++ b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go @@ -2026,6 +2026,13 @@ func TestTenantLogic_udf_star( runLogicTest(t, "udf_star") } +func TestTenantLogic_udf_volatility_check( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_volatility_check") +} + func TestTenantLogic_union( t *testing.T, ) { diff --git a/pkg/sql/alter_function.go b/pkg/sql/alter_function.go index f102af458071..88cc9230c784 100644 --- a/pkg/sql/alter_function.go +++ b/pkg/sql/alter_function.go @@ -77,8 +77,10 @@ func (n *alterFunctionOptionsNode) startExec(params runParams) error { for _, option := range n.n.Options { // Note that language and function body cannot be altered, and it's blocked // from parser level with "common_func_opt_item" syntax. - err := setFuncOption(params, fnDesc, option) - if err != nil { + if err := maybeValidateNewFuncVolatility(params, fnDesc, option); err != nil { + return err + } + if err := setFuncOption(params, fnDesc, option); err != nil { return err } } @@ -101,6 +103,29 @@ func (n *alterFunctionOptionsNode) startExec(params runParams) error { return params.p.logEvent(params.ctx, fnDesc.GetID(), &event) } +func maybeValidateNewFuncVolatility( + params runParams, fnDesc catalog.FunctionDescriptor, option tree.FunctionOption, +) error { + switch t := option.(type) { + case tree.FunctionVolatility: + f := NewReferenceProviderFactory(params.p) + ast, err := fnDesc.ToCreateExpr() + if err != nil { + return err + } + for i, o := range ast.Options { + if _, ok := o.(tree.FunctionVolatility); ok { + ast.Options[i] = t + } + } + if _, err := f.NewReferenceProvider(params.ctx, ast); err != nil { + return err + } + } + + return nil +} + func (n *alterFunctionOptionsNode) Next(params runParams) (bool, error) { return false, nil } func (n *alterFunctionOptionsNode) Values() tree.Datums { return tree.Datums{} } func (n *alterFunctionOptionsNode) Close(ctx context.Context) {} diff --git a/pkg/sql/create_function_test.go b/pkg/sql/create_function_test.go index 6f345014aa18..507a69a0b452 100644 --- a/pkg/sql/create_function_test.go +++ b/pkg/sql/create_function_test.go @@ -60,7 +60,7 @@ CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); ) tDB.Exec(t, ` -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -248,7 +248,7 @@ CREATE SEQUENCE sq2; CREATE VIEW v1 AS SELECT 1; CREATE VIEW v2 AS SELECT 2; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT b FROM t1@t1_idx_b; SELECT a FROM v1; SELECT nextval('sq1'); @@ -292,7 +292,7 @@ SELECT nextval(106:::REGCLASS);`, // Replace the function body with another group of objects and make sure // references are modified correctly. tDB.Exec(t, ` -CREATE OR REPLACE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE OR REPLACE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT b FROM t2@t2_idx_b; SELECT a FROM v2; SELECT nextval('sq2'); diff --git a/pkg/sql/drop_function_test.go b/pkg/sql/drop_function_test.go index d7e697980c66..fcd8e518eb41 100644 --- a/pkg/sql/drop_function_test.go +++ b/pkg/sql/drop_function_test.go @@ -50,7 +50,7 @@ CREATE TABLE t( CREATE SEQUENCE sq1; CREATE VIEW v AS SELECT a FROM t; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -206,7 +206,7 @@ CREATE TABLE t2(a INT PRIMARY KEY); CREATE VIEW v AS SELECT a FROM t2; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); CREATE SCHEMA test_sc; -CREATE FUNCTION test_sc.f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION test_sc.f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -216,7 +216,7 @@ CREATE FUNCTION test_sc.f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$; CREATE DATABASE test_udf_db; USE test_udf_db; -CREATE FUNCTION test_udf_db.public.f() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION test_udf_db.public.f() RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT 1; $$; USE defaultdb; @@ -330,7 +330,7 @@ CREATE TABLE t2(a INT PRIMARY KEY); CREATE VIEW v AS SELECT a FROM t2; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); CREATE SCHEMA test_sc; -CREATE FUNCTION test_sc.f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION test_sc.f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; diff --git a/pkg/sql/function_resolver_test.go b/pkg/sql/function_resolver_test.go index fa3c04a123d4..d3491b55dc8f 100644 --- a/pkg/sql/function_resolver_test.go +++ b/pkg/sql/function_resolver_test.go @@ -56,15 +56,15 @@ CREATE SEQUENCE sq1; CREATE TABLE t2(a INT PRIMARY KEY); CREATE VIEW v AS SELECT a FROM t2; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; SELECT a FROM v; SELECT nextval('sq1'); $$; -CREATE FUNCTION f() RETURNS VOID IMMUTABLE LANGUAGE SQL AS $$ SELECT 1 $$; -CREATE FUNCTION f(INT) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT a FROM t $$; +CREATE FUNCTION f() RETURNS VOID VOLATILE LANGUAGE SQL AS $$ SELECT 1 $$; +CREATE FUNCTION f(INT) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t $$; `) var sessionData sessiondatapb.SessionData @@ -166,9 +166,9 @@ func TestResolveFunctionRespectSearchPath(t *testing.T) { tDB.Exec(t, ` CREATE SCHEMA sc1; CREATE SCHEMA sc2; -CREATE FUNCTION sc1.f() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 1 $$; -CREATE FUNCTION sc2.f() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 2 $$; -CREATE FUNCTION sc1.lower() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 3 $$; +CREATE FUNCTION sc1.f() RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT 1 $$; +CREATE FUNCTION sc2.f() RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT 2 $$; +CREATE FUNCTION sc1.lower() RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT 3 $$; `, ) @@ -306,10 +306,10 @@ func TestFuncExprTypeCheck(t *testing.T) { tDB.Exec(t, ` CREATE SCHEMA sc1; CREATE SCHEMA sc2; -CREATE FUNCTION sc1.f(a INT, b INT) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 1 $$; -CREATE FUNCTION sc1.f(a INT) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 2 $$; -CREATE FUNCTION sc2.f(a INT) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 3 $$; -CREATE FUNCTION sc1.lower(a STRING) RETURNS STRING IMMUTABLE LANGUAGE SQL AS $$ SELECT lower('HI') $$; +CREATE FUNCTION sc1.f(a INT, b INT) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT 1 $$; +CREATE FUNCTION sc1.f(a INT) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT 2 $$; +CREATE FUNCTION sc2.f(a INT) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT 3 $$; +CREATE FUNCTION sc1.lower(a STRING) RETURNS STRING VOLATILE LANGUAGE SQL AS $$ SELECT lower('HI') $$; `, ) diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf index 35613d8437e4..ba0809534d0d 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf +++ b/pkg/sql/logictest/testdata/logic_test/udf @@ -107,7 +107,7 @@ statement ok CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); statement ok -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -119,7 +119,7 @@ SELECT create_statement FROM [SHOW CREATE FUNCTION f]; ---- CREATE FUNCTION public.f(IN a test.public.notmyworkday) RETURNS INT8 - IMMUTABLE + VOLATILE NOT LEAKPROOF CALLED ON NULL INPUT LANGUAGE SQL @@ -652,7 +652,7 @@ public a int8 int8 func public b int8 int8 func volatile public c int8 int8, int8 func volatile public d int4 int2 func volatile -public f int8 notmyworkday func immutable +public f int8 notmyworkday func volatile public f_no_ref int8 int8 func immutable public proc_f int8 int8 func volatile public proc_f text text, int8 func immutable @@ -672,7 +672,7 @@ public a int8 int8 func public b int8 int8 func volatile public c int8 int8, int8 func volatile public d int4 int2 func volatile -public f int8 notmyworkday func immutable +public f int8 notmyworkday func volatile public f_no_ref int8 int8 func immutable public proc_f int8 int8 func volatile public proc_f text text, int8 func immutable @@ -696,7 +696,7 @@ public a int8 int8 func public b int8 int8 func volatile public c int8 int8, int8 func volatile public d int4 int2 func volatile -public f int8 notmyworkday func immutable +public f int8 notmyworkday func volatile public f_no_ref int8 int8 func immutable public proc_f int8 int8 func volatile public proc_f text text, int8 func immutable @@ -2286,10 +2286,10 @@ subtest volatility statement ok CREATE TABLE kv (k INT PRIMARY KEY, v INT); INSERT INTO kv VALUES (1, 1), (2, 2), (3, 3); -CREATE FUNCTION get_l(i INT) RETURNS INT IMMUTABLE LEAKPROOF LANGUAGE SQL AS $$ +CREATE FUNCTION get_l(i INT) RETURNS INT STABLE LANGUAGE SQL AS $$ SELECT v FROM kv WHERE k = i; $$; -CREATE FUNCTION get_i(i INT) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION get_i(i INT) RETURNS INT STABLE LANGUAGE SQL AS $$ SELECT v FROM kv WHERE k = i; $$; CREATE FUNCTION get_s(i INT) RETURNS INT STABLE LANGUAGE SQL AS $$ @@ -2307,8 +2307,8 @@ SELECT pg_get_functiondef('get_l'::regproc::oid) ---- CREATE FUNCTION public.get_l(IN i INT8) RETURNS INT8 - IMMUTABLE - LEAKPROOF + STABLE + NOT LEAKPROOF CALLED ON NULL INPUT LANGUAGE SQL AS $$ @@ -2352,39 +2352,22 @@ l1 l2 i1 i2 s1 s2 v1 v2 statement ok CREATE SEQUENCE sq2; + +statement error volatile statement not allowed in immutable function: SELECT nextval\('sq2'\) CREATE FUNCTION rand_i() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$SELECT nextval('sq2')$$; + +statement error volatile statement not allowed in stable function: 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')$$; -# UDFs with mislabeled volatilities. The volatile functions incorrectly marked -# immutable and stable are evaluated just once, either during optimization or -# execution. -# -# Note: This particular example does not match the behavior of Postgres 14.6 -# exactly, where rand_s() returns a different value in each row. However, -# Postgres can fold these stable functions in some situations. Specifically, -# Postgres will fold a stable function if doing so allows it to plan a -# constrained index scan. For example: -# -# CREATE TABLE t (i INT PRIMARY KEY); -# INSERT INTO t SELECT * FROM generate_series(1, 1000000); -# ANALYZE t; -# CREATE FUNCTION rand_s() RETURNS INT STABLE LANGUAGE SQL AS 'SELECT (10*random())::INT'; -# SELECT i FROM t WHERE i = rand_s(); -# -# The query plan of the SELECT is a contrained index scan, with rand_s() only -# being evalauated once. With this query plan, the SELECT always returns a -# single row. -# -# So, our behavior is more aggressive in folding table functions, but this is -# perfectly valid. -query IIIIII rowsort -SELECT rand_i(), rand_i(), rand_s(), rand_s(), rand_v(), rand_v() FROM generate_series(1, 3) +query II rowsort +SELECT rand_v(), rand_v() FROM generate_series(1, 3) ---- -1 2 3 4 5 6 -1 2 3 4 7 8 -1 2 3 4 9 10 - +1 2 +3 4 +5 6 subtest implicit_record_types @@ -2933,10 +2916,10 @@ SELECT oid, proname, pronamespace, proowner, prolang, proleakproof, proisstrict, FROM pg_catalog.pg_proc WHERE proname IN ('f_93314', 'f_93314_alias', 'f_93314_comp', 'f_93314_comp_t') ORDER BY oid; ---- -100266 f_93314 105 1546506610 14 false false false v 0 100265 · {} NULL SELECT i, e FROM test.public.t_93314 ORDER BY i LIMIT 1; -100268 f_93314_alias 105 1546506610 14 false false false v 0 100267 · {} NULL SELECT i, e FROM test.public.t_93314_alias ORDER BY i LIMIT 1; -100272 f_93314_comp 105 1546506610 14 false false false v 0 100269 · {} NULL SELECT (1, 2); -100273 f_93314_comp_t 105 1546506610 14 false false false v 0 100271 · {} NULL SELECT a, c FROM test.public.t_93314_comp LIMIT 1; +100264 f_93314 105 1546506610 14 false false false v 0 100263 · {} NULL SELECT i, e FROM test.public.t_93314 ORDER BY i LIMIT 1; +100266 f_93314_alias 105 1546506610 14 false false false v 0 100265 · {} NULL SELECT i, e FROM test.public.t_93314_alias ORDER BY i LIMIT 1; +100270 f_93314_comp 105 1546506610 14 false false false v 0 100267 · {} NULL SELECT (1, 2); +100271 f_93314_comp_t 105 1546506610 14 false false false v 0 100269 · {} NULL SELECT a, c FROM test.public.t_93314_comp LIMIT 1; # Regression test for #95240. Strict UDFs that are inlined should result in NULL # when presented with NULL arguments. diff --git a/pkg/sql/logictest/testdata/logic_test/udf_volatility_check b/pkg/sql/logictest/testdata/logic_test/udf_volatility_check new file mode 100644 index 000000000000..0f526d6227d5 --- /dev/null +++ b/pkg/sql/logictest/testdata/logic_test/udf_volatility_check @@ -0,0 +1,98 @@ +statement ok +SET allow_ordinal_column_references=true + +statement ok +CREATE TABLE t1(a INT PRIMARY KEY, b INT); +CREATE TABLE t2(a INT PRIMARY KEY, b INT); + +statement error pq: referencing relations is not allowed in immutable function +CREATE FUNCTION f() RETURNS FLOAT LANGUAGE SQL IMMUTABLE AS $$ SELECT a FROM t1 $$; + +statement error pq: volatile statement not allowed in immutable function: SELECT random\(\) +CREATE FUNCTION f() RETURNS FLOAT LANGUAGE SQL IMMUTABLE AS $$ SELECT random() $$; + +statement error pq: stable statement not allowed in immutable function: SELECT statement_timestamp\(\) +CREATE FUNCTION f() RETURNS TIMESTAMP LANGUAGE SQL IMMUTABLE AS $$ SELECT statement_timestamp() $$; + +statement error pq: volatile statement not allowed in stable function: SELECT random\(\) +CREATE FUNCTION f() RETURNS FLOAT LANGUAGE SQL STABLE AS $$ SELECT random() $$; + +statement error pq: volatile statement not allowed in immutable function: SELECT @1 FROM ROWS FROM \(random\(\)\) +CREATE FUNCTION f() RETURNS FLOAT LANGUAGE SQL IMMUTABLE AS $$ SELECT @1 FROM random() $$; + +statement error pq: volatile statement not allowed in immutable function: SELECT t1\.a FROM test\.public\.t1 JOIN test\.public\.t2 ON t1\.a = \(t2\.a \+ random\(\)::INT8\) +CREATE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ + SELECT t1.a + FROM t1 + JOIN t2 ON t1.a = t2.a + random()::INT +$$; + +statement error pq: volatile statement not allowed in immutable function: SELECT a FROM test\.public\.t1 WHERE b = \(1\.0 \+ random\(\)\) +CREATE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ + SELECT a + FROM t1 + WHERE b = 1 + random() +$$; + +subtest replace_func_volatility + +statement ok +CREATE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT 1 $$; + +statement error pq: referencing relations is not allowed in immutable function +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT a FROM t1 $$; + +statement error pq: volatile statement not allowed in immutable function: SELECT random\(\) +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT random()::INT $$; + +statement error pq: stable statement not allowed in immutable function: SELECT statement_timestamp\(\) +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT statement_timestamp()::INT $$; + +statement error pq: volatile statement not allowed in stable function: SELECT random\(\) +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL STABLE AS $$ SELECT random() $$; + +statement error pq: volatile statement not allowed in immutable function: SELECT @1 FROM ROWS FROM \(random\(\)\) +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT @1 FROM random() $$; + +statement error pq: volatile statement not allowed in immutable function: SELECT t1\.a FROM test\.public\.t1 JOIN test\.public\.t2 ON t1\.a = \(t2\.a \+ random\(\)::INT8\) +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ + SELECT t1.a + FROM t1 + JOIN t2 ON t1.a = t2.a + random()::INT +$$; + +statement error pq: volatile statement not allowed in immutable function: SELECT a FROM test\.public\.t1 WHERE b = \(1\.0 \+ random\(\)\) +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ + SELECT a + FROM t1 + WHERE b = 1 + random() +$$; + +subtest alter_func_volatility + +statement ok +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL IMMUTABLE AS $$ SELECT 1 $$; + +statement ok +ALTER FUNCTION f STABLE + +statement ok +ALTER FUNCTION f VOLATILE + +statement ok +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL STABLE AS $$ SELECT statement_timestamp()::INT $$; + +statement ok +ALTER FUNCTION f VOLATILE + +statement error pq: stable statement not allowed in immutable function: SELECT statement_timestamp\(\)::INT8 +ALTER FUNCTION f IMMUTABLE + +statement ok +CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL VOLATILE AS $$ SELECT random()::INT $$; + +statement error pq: volatile statement not allowed in stable function: SELECT random\(\)::INT8 +ALTER FUNCTION f STABLE + +statement error pq: volatile statement not allowed in immutable function: SELECT random\(\)::INT8 +ALTER FUNCTION f IMMUTABLE diff --git a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go index 1d7d2263459a..d89bcbac28fa 100644 --- a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go @@ -1997,6 +1997,13 @@ func TestLogic_udf_star( runLogicTest(t, "udf_star") } +func TestLogic_udf_volatility_check( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_volatility_check") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go b/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go index bb58d21dbe4e..9882e7ba03ed 100644 --- a/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go @@ -2004,6 +2004,13 @@ func TestLogic_udf_star( runLogicTest(t, "udf_star") } +func TestLogic_udf_volatility_check( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_volatility_check") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/fakedist/generated_test.go b/pkg/sql/logictest/tests/fakedist/generated_test.go index 0580b4334d2a..566d148fd15b 100644 --- a/pkg/sql/logictest/tests/fakedist/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist/generated_test.go @@ -2018,6 +2018,13 @@ func TestLogic_udf_star( runLogicTest(t, "udf_star") } +func TestLogic_udf_volatility_check( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_volatility_check") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go b/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go index f6e62fb6e38f..deecaa255ce6 100644 --- a/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go +++ b/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go @@ -1983,6 +1983,13 @@ func TestLogic_udf_star( runLogicTest(t, "udf_star") } +func TestLogic_udf_volatility_check( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_volatility_check") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-vec-off/generated_test.go b/pkg/sql/logictest/tests/local-vec-off/generated_test.go index f554fe62764b..307d0827490b 100644 --- a/pkg/sql/logictest/tests/local-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/local-vec-off/generated_test.go @@ -2018,6 +2018,13 @@ func TestLogic_udf_star( runLogicTest(t, "udf_star") } +func TestLogic_udf_volatility_check( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_volatility_check") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local/generated_test.go b/pkg/sql/logictest/tests/local/generated_test.go index 87de51ba4005..6ffa3e575365 100644 --- a/pkg/sql/logictest/tests/local/generated_test.go +++ b/pkg/sql/logictest/tests/local/generated_test.go @@ -2207,6 +2207,13 @@ func TestLogic_udf_star( runLogicTest(t, "udf_star") } +func TestLogic_udf_volatility_check( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_volatility_check") +} + func TestLogic_union( t *testing.T, ) { diff --git a/pkg/sql/opt/optbuilder/create_function.go b/pkg/sql/opt/optbuilder/create_function.go index 0f3149e5fd9b..482225cd4999 100644 --- a/pkg/sql/opt/optbuilder/create_function.go +++ b/pkg/sql/opt/optbuilder/create_function.go @@ -133,10 +133,18 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateFunction, inScope *scope) ( panic(err) } + targetVolatility := tree.GetFuncVolatility(cf.Options) // Validate each statement and collect the dependencies. fmtCtx := tree.NewFmtCtx(tree.FmtSimple) for i, stmt := range stmts { - stmtScope := b.buildStmt(stmts[i].AST, nil /* desiredTypes */, bodyScope) + var stmtScope *scope + // We need to disable stable function folding because we want to catch the + // volatility of stable functions. If folded, we only get a scalar and lose + // the volatility. + b.factory.FoldingControl().TemporarilyDisallowStableFolds(func() { + stmtScope = b.buildStmt(stmts[i].AST, nil /* desiredTypes */, bodyScope) + }) + checkStmtVolatility(targetVolatility, stmtScope, stmt.AST) // Format the statements with qualified datasource names. formatFuncBodyStmt(fmtCtx, stmt.AST, i > 0 /* newLine */) @@ -160,6 +168,15 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateFunction, inScope *scope) ( b.schemaTypeDeps = intsets.Fast{} } + if targetVolatility == tree.FunctionImmutable && len(deps) > 0 { + panic( + pgerror.Newf( + pgcode.InvalidParameterValue, + "referencing relations is not allowed in immutable function", + ), + ) + } + // Override the function body so that references are fully qualified. for i, option := range cf.Options { if _, ok := option.(tree.FunctionBodyStr); ok { @@ -270,3 +287,21 @@ func validateReturnType(expected *types.T, cols []scopeColumn) error { return nil } + +func checkStmtVolatility( + expectedVolatility tree.FunctionVolatility, stmtScope *scope, stmt tree.Statement, +) { + switch expectedVolatility { + case tree.FunctionImmutable: + if stmtScope.expr.Relational().VolatilitySet.HasVolatile() { + panic(pgerror.Newf(pgcode.InvalidParameterValue, "volatile statement not allowed in immutable function: %s", stmt.String())) + } + if stmtScope.expr.Relational().VolatilitySet.HasStable() { + panic(pgerror.Newf(pgcode.InvalidParameterValue, "stable statement not allowed in immutable function: %s", stmt.String())) + } + case tree.FunctionStable: + if stmtScope.expr.Relational().VolatilitySet.HasVolatile() { + panic(pgerror.Newf(pgcode.InvalidParameterValue, "volatile statement not allowed in stable function: %s", stmt.String())) + } + } +} diff --git a/pkg/sql/schemachanger/scbuild/testdata/create_function b/pkg/sql/schemachanger/scbuild/testdata/create_function index 3322a1335e48..9b3b1b00a978 100644 --- a/pkg/sql/schemachanger/scbuild/testdata/create_function +++ b/pkg/sql/schemachanger/scbuild/testdata/create_function @@ -13,7 +13,7 @@ CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); ---- build -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -28,7 +28,7 @@ $$; - [[FunctionName:{DescID: 110}, PUBLIC], ABSENT] {functionId: 110, name: f} - [[FunctionVolatility:{DescID: 110}, PUBLIC], ABSENT] - {functionId: 110, volatility: {volatility: IMMUTABLE}} + {functionId: 110, volatility: {volatility: VOLATILE}} - [[Owner:{DescID: 110}, PUBLIC], ABSENT] {descriptorId: 110, owner: root} - [[UserPrivileges:{DescID: 110, Name: admin}, PUBLIC], ABSENT] diff --git a/pkg/sql/schemachanger/scbuild/testdata/drop_function b/pkg/sql/schemachanger/scbuild/testdata/drop_function index cef8822c25ef..2afa116166c5 100644 --- a/pkg/sql/schemachanger/scbuild/testdata/drop_function +++ b/pkg/sql/schemachanger/scbuild/testdata/drop_function @@ -9,7 +9,7 @@ CREATE TABLE t( CREATE SEQUENCE sq1; CREATE VIEW v AS SELECT a FROM t; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -34,7 +34,7 @@ DROP FUNCTION f; - [[FunctionName:{DescID: 109}, ABSENT], PUBLIC] {functionId: 109, name: f} - [[FunctionVolatility:{DescID: 109}, ABSENT], PUBLIC] - {functionId: 109, volatility: {volatility: IMMUTABLE}} + {functionId: 109, volatility: {volatility: VOLATILE}} - [[FunctionLeakProof:{DescID: 109}, ABSENT], PUBLIC] {functionId: 109} - [[FunctionNullInputBehavior:{DescID: 109}, ABSENT], PUBLIC] diff --git a/pkg/sql/schemachanger/scdecomp/testdata/function b/pkg/sql/schemachanger/scdecomp/testdata/function index b95e8ac6c3a2..8cc06d5c0c6b 100644 --- a/pkg/sql/schemachanger/scdecomp/testdata/function +++ b/pkg/sql/schemachanger/scdecomp/testdata/function @@ -10,7 +10,7 @@ CREATE SEQUENCE sq1; CREATE TABLE t2(a INT PRIMARY KEY); CREATE VIEW v AS SELECT a FROM t2; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -98,7 +98,7 @@ ElementState: - FunctionVolatility: functionId: 110 volatility: - volatility: IMMUTABLE + volatility: VOLATILE Status: PUBLIC - FunctionLeakProof: functionId: 110 diff --git a/pkg/sql/schemachanger/scplan/testdata/create_function b/pkg/sql/schemachanger/scplan/testdata/create_function index 6ada49d0e5a4..9e970122524e 100644 --- a/pkg/sql/schemachanger/scplan/testdata/create_function +++ b/pkg/sql/schemachanger/scplan/testdata/create_function @@ -12,7 +12,7 @@ CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); ---- ops -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -63,7 +63,7 @@ StatementPhase stage 1 of 1 with 10 MutationType ops Name: f *scop.SetFunctionVolatility FunctionID: 109 - Volatility: 2 + Volatility: 1 *scop.UpdateOwner Owner: DescriptorID: 109 @@ -199,7 +199,7 @@ PreCommitPhase stage 2 of 2 with 11 MutationType ops Name: f *scop.SetFunctionVolatility FunctionID: 109 - Volatility: 2 + Volatility: 1 *scop.UpdateOwner Owner: DescriptorID: 109 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_function b/pkg/sql/schemachanger/scplan/testdata/drop_function index 7855e05fba45..f8dc98d2dd47 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_function +++ b/pkg/sql/schemachanger/scplan/testdata/drop_function @@ -9,7 +9,7 @@ CREATE TABLE t( CREATE SEQUENCE sq1; CREATE VIEW v AS SELECT a FROM t; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; diff --git a/pkg/sql/schemachanger/testdata/end_to_end/create_function b/pkg/sql/schemachanger/testdata/end_to_end/create_function index fea4587302bd..0b9465c78873 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/create_function +++ b/pkg/sql/schemachanger/testdata/end_to_end/create_function @@ -20,7 +20,7 @@ CREATE TABLE t2(a notmyworkday); +object {100 101 t2} -> 109 test -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -80,7 +80,7 @@ upsert descriptor #110 + width: 64 + state: ADD + version: "1" - + volatility: IMMUTABLE + + volatility: VOLATILE upsert descriptor #101 schema: + functions: @@ -222,7 +222,7 @@ upsert descriptor #110 + oid: 20 + width: 64 + version: "1" - + volatility: IMMUTABLE + + volatility: VOLATILE upsert descriptor #101 schema: + functions: diff --git a/pkg/sql/schemachanger/testdata/end_to_end/drop_function b/pkg/sql/schemachanger/testdata/end_to_end/drop_function index 9ecf429a857e..6c089bb66950 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/drop_function +++ b/pkg/sql/schemachanger/testdata/end_to_end/drop_function @@ -9,7 +9,7 @@ CREATE TABLE t( CREATE SEQUENCE sq1; CREATE VIEW v AS SELECT a FROM t; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -131,7 +131,7 @@ upsert descriptor #109 - version: "1" + state: DROP + version: "2" - volatility: IMMUTABLE + volatility: VOLATILE # end StatementPhase # begin PreCommitPhase ## PreCommitPhase stage 1 of 2 with 1 MutationType op @@ -283,7 +283,7 @@ upsert descriptor #109 - version: "1" + state: DROP + version: "2" - volatility: IMMUTABLE + volatility: VOLATILE persist all catalog changes to storage create job #1 (non-cancelable: true): "DROP FUNCTION \"\".\"\".f" descriptor IDs: [104 105 106 107 108 109] diff --git a/pkg/sql/schemachanger/testdata/explain/create_function b/pkg/sql/schemachanger/testdata/explain/create_function index 3c9c2cfb55ca..f59654275b89 100644 --- a/pkg/sql/schemachanger/testdata/explain/create_function +++ b/pkg/sql/schemachanger/testdata/explain/create_function @@ -12,7 +12,7 @@ CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); CREATE TABLE t2(a notmyworkday); /* test */ -EXPLAIN (ddl) CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +EXPLAIN (ddl) CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -22,7 +22,7 @@ $$; ---- Schema change plan for CREATE FUNCTION ‹defaultdb›.‹public›.‹f›(IN ‹a› notmyworkday) RETURNS INT8 - IMMUTABLE + VOLATILE LANGUAGE SQL AS $$SELECT a FROM defaultdb.public.t; SELECT b FROM defaultdb.public.t@t_idx_b; @@ -43,7 +43,7 @@ SELECT nextval('sq1');$$; │ └── 10 Mutation operations │ ├── CreateFunctionDescriptor {"Function":{"FunctionID":110}} │ ├── SetFunctionName {"FunctionID":110,"Name":"f"} - │ ├── SetFunctionVolatility {"FunctionID":110,"Volatility":2} + │ ├── SetFunctionVolatility {"FunctionID":110,"Volatility":1} │ ├── UpdateOwner {"Owner":{"DescriptorID":110,"Owner":"root"}} │ ├── UpdateUserPrivileges {"Privileges":{"DescriptorID":110,"Privileges":2,"UserName":"admin","WithGrantOption":2}} │ ├── UpdateUserPrivileges {"Privileges":{"DescriptorID":110,"Privileges":2,"UserName":"root","WithGrantOption":2}} @@ -77,7 +77,7 @@ SELECT nextval('sq1');$$; └── 11 Mutation operations ├── CreateFunctionDescriptor {"Function":{"FunctionID":110}} ├── SetFunctionName {"FunctionID":110,"Name":"f"} - ├── SetFunctionVolatility {"FunctionID":110,"Volatility":2} + ├── SetFunctionVolatility {"FunctionID":110,"Volatility":1} ├── UpdateOwner {"Owner":{"DescriptorID":110,"Owner":"root"}} ├── UpdateUserPrivileges {"Privileges":{"DescriptorID":110,"Privileges":2,"UserName":"admin","WithGrantOption":2}} ├── UpdateUserPrivileges {"Privileges":{"DescriptorID":110,"Privileges":2,"UserName":"root","WithGrantOption":2}} diff --git a/pkg/sql/schemachanger/testdata/explain/drop_function b/pkg/sql/schemachanger/testdata/explain/drop_function index 9e9962cb918f..b987e3cda99d 100644 --- a/pkg/sql/schemachanger/testdata/explain/drop_function +++ b/pkg/sql/schemachanger/testdata/explain/drop_function @@ -9,7 +9,7 @@ CREATE TABLE t( CREATE SEQUENCE sq1; CREATE VIEW v AS SELECT a FROM t; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/create_function b/pkg/sql/schemachanger/testdata/explain_verbose/create_function index 12047de4dfae..92601006a44f 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/create_function +++ b/pkg/sql/schemachanger/testdata/explain_verbose/create_function @@ -12,7 +12,7 @@ CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); CREATE TABLE t2(a notmyworkday); /* test */ -EXPLAIN (ddl, verbose) CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +EXPLAIN (ddl, verbose) CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; @@ -22,7 +22,7 @@ $$; ---- • Schema change plan for CREATE FUNCTION ‹defaultdb›.‹public›.‹f›(IN ‹a› notmyworkday) │ RETURNS INT8 -│ IMMUTABLE +│ VOLATILE │ LANGUAGE SQL │ AS $$SELECT a FROM defaultdb.public.t; │ SELECT b FROM defaultdb.public.t@t_idx_b; @@ -120,7 +120,7 @@ $$; │ │ │ ├── • SetFunctionVolatility │ │ FunctionID: 110 -│ │ Volatility: 2 +│ │ Volatility: 1 │ │ │ ├── • UpdateOwner │ │ Owner: @@ -352,7 +352,7 @@ $$; │ ├── • SetFunctionVolatility │ FunctionID: 110 - │ Volatility: 2 + │ Volatility: 1 │ ├── • UpdateOwner │ Owner: diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/drop_function b/pkg/sql/schemachanger/testdata/explain_verbose/drop_function index 2bee9fce9d14..27ce4cdcfd94 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/drop_function +++ b/pkg/sql/schemachanger/testdata/explain_verbose/drop_function @@ -9,7 +9,7 @@ CREATE TABLE t( CREATE SEQUENCE sq1; CREATE VIEW v AS SELECT a FROM t; CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); -CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ +CREATE FUNCTION f(a notmyworkday) RETURNS INT VOLATILE LANGUAGE SQL AS $$ SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; diff --git a/pkg/sql/sem/tree/udf.go b/pkg/sql/sem/tree/udf.go index d737347a05cf..c58a7b165eef 100644 --- a/pkg/sql/sem/tree/udf.go +++ b/pkg/sql/sem/tree/udf.go @@ -560,3 +560,16 @@ func ValidateFuncOptions(options FunctionOptions) error { return nil } + +// GetFuncVolatility tries to find a function volatility from the given list of +// function options. If there is no volatility found, FunctionVolatile is +// returned as the default. +func GetFuncVolatility(options FunctionOptions) FunctionVolatility { + for _, option := range options { + switch t := option.(type) { + case FunctionVolatility: + return t + } + } + return FunctionVolatile +} From 27df3cb4fd15bcb3570840553af074e378b22d37 Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Mon, 6 Feb 2023 11:33:59 -0500 Subject: [PATCH 03/10] storage: fix TestPebbleMetricEventListener In #96021 handling of disk slow events was made asynchronous. This introduced a race during Close where the goroutine logging could still be inflight. Fix #96635. Epic: None Release note: None --- pkg/storage/pebble.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/storage/pebble.go b/pkg/storage/pebble.go index e555d148fea3..f8fbe89f2607 100644 --- a/pkg/storage/pebble.go +++ b/pkg/storage/pebble.go @@ -735,6 +735,7 @@ type Pebble struct { syncutil.Mutex flushCompletedCallback func() } + asyncDone sync.WaitGroup // supportsRangeKeys is 1 if the database supports range keys. It must // be accessed atomically. @@ -1006,7 +1007,7 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) { oldDiskSlow := lel.DiskSlow lel.DiskSlow = func(info pebble.DiskSlowInfo) { // Run oldDiskSlow asynchronously. - go oldDiskSlow(info) + p.async(func() { oldDiskSlow(info) }) } el := pebble.TeeEventListener( p.makeMetricEtcEventListener(ctx), @@ -1087,6 +1088,17 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) { return p, nil } +// async launches the provided function in a new goroutine. It uses a wait group +// to synchronize with (*Pebble).Close to ensure all launched goroutines have +// exited before Close returns. +func (p *Pebble) async(fn func()) { + p.asyncDone.Add(1) + go func() { + defer p.asyncDone.Done() + fn() + }() +} + func (p *Pebble) makeMetricEtcEventListener(ctx context.Context) pebble.EventListener { return pebble.EventListener{ WriteStallBegin: func(info pebble.WriteStallBeginInfo) { @@ -1132,7 +1144,7 @@ func (p *Pebble) makeMetricEtcEventListener(ctx context.Context) pebble.EventLis log.Fatalf(ctx, "file write stall detected: %s", info) } else { - go log.Errorf(ctx, "file write stall detected: %s", info) + p.async(func() { log.Errorf(ctx, "file write stall detected: %s", info) }) } return } @@ -1172,6 +1184,9 @@ func (p *Pebble) Close() { } p.closed = true + // Wait for any asynchronous goroutines to exit. + p.asyncDone.Wait() + handleErr := func(err error) { if err == nil { return From b3041b912509881fac5447d2dc47e8bee6a3c092 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 13 Jan 2023 23:12:21 +0100 Subject: [PATCH 04/10] sem/tree: avoid heap allocs in UDF pretty-print Release note: None --- pkg/sql/alter_function.go | 4 ++-- pkg/sql/sem/tree/udf.go | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/sql/alter_function.go b/pkg/sql/alter_function.go index cbf353bbab58..d7fb8a35e9ff 100644 --- a/pkg/sql/alter_function.go +++ b/pkg/sql/alter_function.go @@ -148,7 +148,7 @@ func (n *alterFunctionRenameNode) startExec(params runParams) error { if existing != nil { return pgerror.Newf( pgcode.DuplicateFunction, "function %s already exists in schema %q", - tree.AsString(maybeExistingFuncObj), scDesc.GetName(), + tree.AsString(&maybeExistingFuncObj), scDesc.GetName(), ) } @@ -312,7 +312,7 @@ func (n *alterFunctionSetSchemaNode) startExec(params runParams) error { if existing != nil { return pgerror.Newf( pgcode.DuplicateFunction, "function %s already exists in schema %q", - tree.AsString(maybeExistingFuncObj), targetSc.GetName(), + tree.AsString(&maybeExistingFuncObj), targetSc.GetName(), ) } diff --git a/pkg/sql/sem/tree/udf.go b/pkg/sql/sem/tree/udf.go index d737347a05cf..b887f987f816 100644 --- a/pkg/sql/sem/tree/udf.go +++ b/pkg/sql/sem/tree/udf.go @@ -280,11 +280,11 @@ type FuncParams []FuncParam // Format implements the NodeFormatter interface. func (node FuncParams) Format(ctx *FmtCtx) { - for i, arg := range node { + for i := range node { if i > 0 { ctx.WriteString(", ") } - ctx.FormatNode(&arg) + ctx.FormatNode(&node[i]) } } @@ -367,11 +367,11 @@ type FuncObjs []FuncObj // Format implements the NodeFormatter interface. func (node FuncObjs) Format(ctx *FmtCtx) { - for i, f := range node { + for i := range node { if i > 0 { ctx.WriteString(", ") } - ctx.FormatNode(f) + ctx.FormatNode(&node[i]) } } @@ -382,7 +382,7 @@ type FuncObj struct { } // Format implements the NodeFormatter interface. -func (node FuncObj) Format(ctx *FmtCtx) { +func (node *FuncObj) Format(ctx *FmtCtx) { ctx.FormatNode(&node.FuncName) if node.Params != nil { ctx.WriteString("(") @@ -419,7 +419,7 @@ type AlterFunctionOptions struct { // Format implements the NodeFormatter interface. func (node *AlterFunctionOptions) Format(ctx *FmtCtx) { ctx.WriteString("ALTER FUNCTION ") - ctx.FormatNode(node.Function) + ctx.FormatNode(&node.Function) for _, option := range node.Options { ctx.WriteString(" ") ctx.FormatNode(option) @@ -435,7 +435,7 @@ type AlterFunctionRename struct { // Format implements the NodeFormatter interface. func (node *AlterFunctionRename) Format(ctx *FmtCtx) { ctx.WriteString("ALTER FUNCTION ") - ctx.FormatNode(node.Function) + ctx.FormatNode(&node.Function) ctx.WriteString(" RENAME TO ") ctx.WriteString(string(node.NewName)) } @@ -449,7 +449,7 @@ type AlterFunctionSetSchema struct { // Format implements the NodeFormatter interface. func (node *AlterFunctionSetSchema) Format(ctx *FmtCtx) { ctx.WriteString("ALTER FUNCTION ") - ctx.FormatNode(node.Function) + ctx.FormatNode(&node.Function) ctx.WriteString(" SET SCHEMA ") ctx.WriteString(string(node.NewSchemaName)) } @@ -463,7 +463,7 @@ type AlterFunctionSetOwner struct { // Format implements the NodeFormatter interface. func (node *AlterFunctionSetOwner) Format(ctx *FmtCtx) { ctx.WriteString("ALTER FUNCTION ") - ctx.FormatNode(node.Function) + ctx.FormatNode(&node.Function) ctx.WriteString(" OWNER TO ") ctx.FormatNode(&node.NewOwner) } @@ -478,7 +478,7 @@ type AlterFunctionDepExtension struct { // Format implements the NodeFormatter interface. func (node *AlterFunctionDepExtension) Format(ctx *FmtCtx) { ctx.WriteString("ALTER FUNCTION ") - ctx.FormatNode(node.Function) + ctx.FormatNode(&node.Function) if node.Remove { ctx.WriteString(" NO") } From bafc83d3ae0775f1f5f205260394b1c0c68fa8c3 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 13 Jan 2023 23:18:29 +0100 Subject: [PATCH 05/10] sem/tree: avoid heap allocations when pretty-printing roles Release note: None --- pkg/sql/sem/tree/alter_default_privileges.go | 4 ++-- pkg/sql/sem/tree/show.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/sql/sem/tree/alter_default_privileges.go b/pkg/sql/sem/tree/alter_default_privileges.go index 9371613451b9..b346a9d7be31 100644 --- a/pkg/sql/sem/tree/alter_default_privileges.go +++ b/pkg/sql/sem/tree/alter_default_privileges.go @@ -40,11 +40,11 @@ func (n *AlterDefaultPrivileges) Format(ctx *FmtCtx) { ctx.WriteString("FOR ALL ROLES ") } else if len(n.Roles) > 0 { ctx.WriteString("FOR ROLE ") - for i, role := range n.Roles { + for i := range n.Roles { if i > 0 { ctx.WriteString(", ") } - ctx.FormatNode(&role) + ctx.FormatNode(&n.Roles[i]) } ctx.WriteString(" ") } diff --git a/pkg/sql/sem/tree/show.go b/pkg/sql/sem/tree/show.go index 3a8d515b8bc8..2bb509a7b686 100644 --- a/pkg/sql/sem/tree/show.go +++ b/pkg/sql/sem/tree/show.go @@ -1231,11 +1231,11 @@ func (n *ShowDefaultPrivileges) Format(ctx *FmtCtx) { ctx.WriteString("SHOW DEFAULT PRIVILEGES ") if len(n.Roles) > 0 { ctx.WriteString("FOR ROLE ") - for i, role := range n.Roles { + for i := range n.Roles { if i > 0 { ctx.WriteString(", ") } - ctx.FormatNode(&role) + ctx.FormatNode(&n.Roles[i]) } ctx.WriteString(" ") } else if n.ForAllRoles { From 5169600be0fef25bb93f7b202f6cabcc07c3ba19 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 13 Jan 2023 23:19:06 +0100 Subject: [PATCH 06/10] sem/tree: avoid heap allocs when pretty-printing changefeed targets Release note: None --- pkg/sql/sem/tree/changefeed.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/sql/sem/tree/changefeed.go b/pkg/sql/sem/tree/changefeed.go index 81038eab6b51..355391c5657a 100644 --- a/pkg/sql/sem/tree/changefeed.go +++ b/pkg/sql/sem/tree/changefeed.go @@ -89,11 +89,11 @@ type ChangefeedTargets []ChangefeedTarget // Format implements the NodeFormatter interface. func (cts *ChangefeedTargets) Format(ctx *FmtCtx) { - for i, ct := range *cts { + for i := range *cts { if i > 0 { ctx.WriteString(", ") } - ctx.FormatNode(&ct) + ctx.FormatNode(&(*cts)[i]) } } From 6cdeeb3dcaad6b3608818b05206d490de6613fce Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 13 Jan 2023 23:19:45 +0100 Subject: [PATCH 07/10] sem/tree: avoid heap allocs when pretty-printing tuple labels Release note: None --- pkg/sql/sem/tree/pretty.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/sql/sem/tree/pretty.go b/pkg/sql/sem/tree/pretty.go index 0f2b2f3c4381..f72bd9c8b7ae 100644 --- a/pkg/sql/sem/tree/pretty.go +++ b/pkg/sql/sem/tree/pretty.go @@ -1121,8 +1121,9 @@ func (node *Tuple) doc(p *PrettyCfg) pretty.Doc { d := p.bracket("(", exprDoc, ")") if len(node.Labels) > 0 { labels := make([]pretty.Doc, len(node.Labels)) - for i, n := range node.Labels { - labels[i] = p.Doc((*Name)(&n)) + for i := range node.Labels { + n := &node.Labels[i] + labels[i] = p.Doc((*Name)(n)) } d = p.bracket("(", pretty.Stack( d, From 484d9cca3aef18851f18e3c1b572a0f713a2fba7 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 13 Jan 2023 23:20:14 +0100 Subject: [PATCH 08/10] sem/tree: avoid heap allocs when pretty-printing super regions Release note: None --- pkg/sql/sem/tree/alter_database.go | 8 ++++---- pkg/sql/sem/tree/create.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/sql/sem/tree/alter_database.go b/pkg/sql/sem/tree/alter_database.go index 83641de00d61..95cd85f412cb 100644 --- a/pkg/sql/sem/tree/alter_database.go +++ b/pkg/sql/sem/tree/alter_database.go @@ -131,11 +131,11 @@ func (node *AlterDatabaseAddSuperRegion) Format(ctx *FmtCtx) { ctx.WriteString(" ADD SUPER REGION ") ctx.FormatNode(&node.SuperRegionName) ctx.WriteString(" VALUES ") - for i, region := range node.Regions { + for i := range node.Regions { if i != 0 { ctx.WriteString(",") } - ctx.FormatNode(®ion) + ctx.FormatNode(&node.Regions[i]) } } @@ -173,11 +173,11 @@ func (node *AlterDatabaseAlterSuperRegion) Format(ctx *FmtCtx) { ctx.WriteString(" ALTER SUPER REGION ") ctx.FormatNode(&node.SuperRegionName) ctx.WriteString(" VALUES ") - for i, region := range node.Regions { + for i := range node.Regions { if i != 0 { ctx.WriteString(",") } - ctx.FormatNode(®ion) + ctx.FormatNode(&node.Regions[i]) } } diff --git a/pkg/sql/sem/tree/create.go b/pkg/sql/sem/tree/create.go index 6211ce627729..efff6c7f20cf 100644 --- a/pkg/sql/sem/tree/create.go +++ b/pkg/sql/sem/tree/create.go @@ -2228,10 +2228,10 @@ func (node *SuperRegion) Format(ctx *FmtCtx) { ctx.WriteString(" SUPER REGION ") ctx.FormatNode(&node.Name) ctx.WriteString(" VALUES ") - for i, region := range node.Regions { + for i := range node.Regions { if i != 0 { ctx.WriteString(",") } - ctx.FormatNode(®ion) + ctx.FormatNode(&node.Regions[i]) } } From 5b19c6c5ff9199dc8cd6a129e0014b95fb61a653 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 13 Jan 2023 23:20:44 +0100 Subject: [PATCH 09/10] sem/tree: avoid heap allocs when pretty-printing composite type defs Release note: None --- pkg/sql/sem/tree/create.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/sql/sem/tree/create.go b/pkg/sql/sem/tree/create.go index efff6c7f20cf..fc5ddab62cef 100644 --- a/pkg/sql/sem/tree/create.go +++ b/pkg/sql/sem/tree/create.go @@ -381,7 +381,8 @@ func (node *CreateType) Format(ctx *FmtCtx) { ctx.WriteString(")") case Composite: ctx.WriteString("AS (") - for i, elem := range node.CompositeTypeList { + for i := range node.CompositeTypeList { + elem := &node.CompositeTypeList[i] if i != 0 { ctx.WriteString(", ") } From abd36000968093794aefe384d03aca278990071f Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Mon, 6 Feb 2023 20:20:03 +0100 Subject: [PATCH 10/10] funcdesc: make `ToFuncObj` return an object on the heap directly Release note: None --- pkg/sql/alter_function.go | 8 ++++---- pkg/sql/catalog/funcdesc/func_desc.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/sql/alter_function.go b/pkg/sql/alter_function.go index d7fb8a35e9ff..744fc60ae1c7 100644 --- a/pkg/sql/alter_function.go +++ b/pkg/sql/alter_function.go @@ -140,7 +140,7 @@ func (n *alterFunctionRenameNode) startExec(params runParams) error { maybeExistingFuncObj := fnDesc.ToFuncObj() maybeExistingFuncObj.FuncName.ObjectName = n.n.NewName - existing, err := params.p.matchUDF(params.ctx, &maybeExistingFuncObj, false /* required */) + existing, err := params.p.matchUDF(params.ctx, maybeExistingFuncObj, false /* required */) if err != nil { return err } @@ -148,7 +148,7 @@ func (n *alterFunctionRenameNode) startExec(params runParams) error { if existing != nil { return pgerror.Newf( pgcode.DuplicateFunction, "function %s already exists in schema %q", - tree.AsString(&maybeExistingFuncObj), scDesc.GetName(), + tree.AsString(maybeExistingFuncObj), scDesc.GetName(), ) } @@ -305,14 +305,14 @@ func (n *alterFunctionSetSchemaNode) startExec(params runParams) error { maybeExistingFuncObj := fnDesc.ToFuncObj() maybeExistingFuncObj.FuncName.SchemaName = tree.Name(targetSc.GetName()) maybeExistingFuncObj.FuncName.ExplicitSchema = true - existing, err := params.p.matchUDF(params.ctx, &maybeExistingFuncObj, false /* required */) + existing, err := params.p.matchUDF(params.ctx, maybeExistingFuncObj, false /* required */) if err != nil { return err } if existing != nil { return pgerror.Newf( pgcode.DuplicateFunction, "function %s already exists in schema %q", - tree.AsString(&maybeExistingFuncObj), targetSc.GetName(), + tree.AsString(maybeExistingFuncObj), targetSc.GetName(), ) } diff --git a/pkg/sql/catalog/funcdesc/func_desc.go b/pkg/sql/catalog/funcdesc/func_desc.go index e1ec17b1a63b..d20b79e51ded 100644 --- a/pkg/sql/catalog/funcdesc/func_desc.go +++ b/pkg/sql/catalog/funcdesc/func_desc.go @@ -504,8 +504,8 @@ func (desc *Mutable) SetParentSchemaID(id descpb.ID) { } // ToFuncObj converts the descriptor to a tree.FuncObj. -func (desc *immutable) ToFuncObj() tree.FuncObj { - ret := tree.FuncObj{ +func (desc *immutable) ToFuncObj() *tree.FuncObj { + ret := &tree.FuncObj{ FuncName: tree.MakeFunctionNameFromPrefix(tree.ObjectNamePrefix{}, tree.Name(desc.Name)), Params: make(tree.FuncParams, len(desc.Params)), }