Hi
On Fri, Feb 7, 2025 at 3:25 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
Hi
The following review is based on v20250117.
pg_dump order seems not right.
CREATE FUNCTION public.test11(text) RETURNS text
LANGUAGE sql
AS $$select v4 $$;
CREATE VARIABLE public.v4 AS text;
first dump function then variable. restore would fail.
we should first dump variables then function.
probably placed it right after CREATE DOMAIN/CREATE TYPE
I cannot repeat this issue. The import should to work, because dump contains `SET check_function_bodies = false;`
The order is designed to support default values, and the default value can be a function, so the variables should be
dumped after functions.
I tested the new syntax too. And you can see, the order for new syntax is changed due dependencies
SET check_function_bodies = false;
SET xmloption = content;
SET client_min_messages = warning;
SET row_security = off;
--
-- Name: fx1(); Type: FUNCTION; Schema: public; Owner: pavel
--
CREATE FUNCTION public.fx1() RETURNS integer
LANGUAGE sql
AS $$ select v1 $$;
ALTER FUNCTION public.fx1() OWNER TO pavel;
--
-- Name: v1; Type: VARIABLE; Schema: public; Owner: pavel
--
CREATE VARIABLE public.v1 AS integer;
ALTER VARIABLE public.v1 OWNER TO pavel;
--
-- Name: fx3(); Type: FUNCTION; Schema: public; Owner: pavel
--
CREATE FUNCTION public.fx3() RETURNS integer
LANGUAGE sql
RETURN public.v1;
ALTER FUNCTION public.fx3() OWNER TO pavel;
drop table if exists t3;
create variable v4 as text;
let v4 = 'hello';
CREATE TABLE t3 (a timestamp, v4 text);
INSERT INTO t3 SELECT i FROM generate_series('2020-01-01'::timestamp,
'2020-12-31'::timestamp,
'10 minute'::interval) s(i);
ANALYZE t3;
create or replace function test11(text) returns text as $$select v4 $$
language sql;
CREATE STATISTICS s4 (ndistinct) ON test11(v4), test11(v4 || 'h') FROM t3;
this "CREATE STATISTICS s4..." should error out?
any objects built on top of functions that use variables should be
marked as volatile.
and we should also consider the implications of it.
There is not any request so expression of statistics should be immutable, although it makes sense (for me).
(2025-02-11 07:48:28) postgres=# create table t4(a int, b int);
CREATE TABLE
(2025-02-11 07:52:32) postgres=# create statistics s5 (ndistinct) on a, (b * (random()*10)::int) from t4;
CREATE STATISTICS
The access to variables in the query is stable (when it is not wrapped by volatile functions - because variables
are passed as query parameters to the queries.
I think it is working correctly
(2025-02-11 07:54:58) postgres=# create or replace function fx20() returns int as $$ let x = x + 1; select x $$ language sql;
CREATE FUNCTION
(2025-02-11 07:55:49) postgres=# let x = 0;
LET
(2025-02-11 07:55:57) postgres=# select x, fx20(), i from generate_series(1,3) g(i);
┌───┬──────┬───┐
│ x │ fx20 │ i │
╞═══╪══════╪═══╡
│ 0 │ 1 │ 1 │
│ 0 │ 2 │ 2 │
│ 0 │ 3 │ 3 │
└───┴──────┴───┘
(3 rows)
I rewrote the patch 09 - the forsing usage of variable fences (VARIABLE(varname)). There are two possible concepts:
1. controlling visibility of session variables - the variables without fencing are visible somewhere, inside fencing are visible everywhere
2. forcing usage of variable fence syntax and raising an error when the variable is used without fence.
Originally I implemented @2, but the disadvantage can be a lot of errors (a lot of warnings related to shadowed variables), so this is not a good default due to the possibility of a lot of writes to log. On the other hand, there are mystic hidden variable issues. Concept @1 can be more simple for gentle introducing of variables. And I can believe it can be a good safe default - the variables are implicitly visible only inside stored procedures, outside stored procedures, the variable fencing should be used, and without variable fence - just the variable is not accessible (so there is not an issue with shadowing warning). The code change between @1 and @2 concepts are just the change of two lines of code. The concept @1 is a little bit similar to search path (where using variable fencing is like using a qualified name). So I think it is closer to current postgres concepts.
Regards
Pavel