Merge #95245 #96476 #96515 #96651

95245: sem/tree: avoid heap allocs during pretty-printing r=chengxiong-ruan,rafiss a=knz

Everywhere applicable, change
```go
  for i, xxx := range yyy {
      ctx.FormatNode(&xxx)
  }
```
to:
```go
  for i := range yyy {
      ctx.FormatNode(&yyy[i])
  }
```

Epic: None

96476: sql: validate function volatility r=chengxiong-ruan a=chengxiong-ruan

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. Stable function folding is turned off
when building statements in CREATE FUNCTION to catch stable
function's 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.

Informs: #87699

96515: ui: fix time scale selection r=maryliag a=maryliag

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

96651: storage: fix TestPebbleMetricEventListener r=itsbilal a=jbowens

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

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>

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)           {}
@@ -140,7 +165,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
 	}
@@ -305,7 +330,7 @@ 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
 	}

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)),
 	}

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');

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;

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') $$;
 `,
 	)
 

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.