Skip to content

Commit

Permalink
sql: udf volatility check
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@chengxiong-ruan
chengxiong-ruan committed Feb 3, 2023
1 parent 1708ea4 commit b9b9ec4
Show file tree
Hide file tree
Showing 26 changed files with 290 additions and 87 deletions.
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 27 additions & 2 deletions pkg/sql/alter_function.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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) {}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/create_function_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/drop_function_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 10 additions & 10 deletions pkg/sql/function_resolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 $$;
`,
)

Expand Down Expand Up @@ -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') $$;
`,
)

Expand Down
65 changes: 24 additions & 41 deletions pkg/sql/logictest/testdata/logic_test/udf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 $$
Expand All @@ -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 $$
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit b9b9ec4

Please sign in to comment.