RE: proposal: schema variables

Hello Pavel

I tested your patch again and I think things are better now, close to perfect for me.

1) Patch review

- We can define CONSTANTs with CREATE IMMUTABLE VARIABLE … I’m really pleased with this

- The previous bug I mentioned to you by private mail (see detail below) has been fixed and a non-regression test case has been added for it.

- I’m now able to export a 12.1 database using pg_dump from the latest git HEAD (internal version 130000).

NB: the condition is “if internal_version < 130000 => don’t try to export any schema variable”.

Also I was able to test a use case for a complex Oracle package I migrated to PostgreSQL (it has a total of 194 variables and constants in it !).
The Oracle package has been migrated to a PostgreSQL schema with one routine per Oracle subprogram.
I’m able to run my business test case using schema variables on those routines and it’s giving me the expected result.

NB: Previous bug was
database1=> CREATE VARIABLE T0_var AS char(14);
CREATE VARIABLE
database1=> CREATE IMMUTABLE VARIABLE Taille_var AS numeric DEFAULT 14;
CREATE VARIABLE
database1=> LET T0_var = LPAD('999', trunc(Taille_var::NUMERIC)::INTEGER, '0');
ERROR: schema variable "taille_var" is declared IMMUTABLE
database1=> LET T0_var = LPAD('999', trunc(Taille_var::NUMERIC)::INTEGER, '0');
ERROR: variable "taille_var" has not valid content

ð It’s now fixed !

2) Regarding documentation

It’s pretty good except some small details :

- sql-createvariable.html => IMMUTABLE parameter : The value of the variable cannot be changed. => I think an article is needed here (the)

- sql-createvariable.html => ON COMMIT DROP : The ON COMMIT DROP clause specifies the bahaviour of temporary => behaviour
Also there are “tabs” between words (here between “of” and “temporary”), making the paragraph look strange.

- sql-createvariable.html => Maybe we should mention that the IMMUTABLE parameter (CREATE IMMUTABLE VARIABLE …) is the way to define global CONSTANTs, because people will look for a way to create global constants in the documentation and it would be good if they can find the CONSTANT word in it.
Also maybe it’s worth mentioning that “schema variables” relates to “global variables” (for the same purpose of people searching in the documentation).

- sql-altervariable.html => sentence “These restrictions enforce that altering the owner doesn't do anything you couldn't do by dropping and recreating the variable.“ => not sure I understand this one. Isn’t it “does not do anything you could do” instead of “you couln’t do” .. but maybe it’s me

Otherwise, this is a really nice feature and I’m looking forward to have it in PostgreSQL.

Thanks a lot

Duval Rémi

De : Pavel Stehule [mailto:pavel(dot)stehule(at)gmail(dot)com]
Envoyé : jeudi 5 mars 2020 18:54
À : Asif Rehman <asifr(dot)rehman(at)gmail(dot)com>
Cc : DUVAL REMI <REMI(dot)DUVAL(at)CHEOPS(dot)FR>; PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Objet : Re: proposal: schema variables

čt 5. 3. 2020 v 15:11 odesílatel Asif Rehman <asifr(dot)rehman(at)gmail(dot)com<mailto:asifr(dot)rehman(at)gmail(dot)com>> napsal:

On Sat, Feb 29, 2020 at 2:10 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com<mailto:pavel(dot)stehule(at)gmail(dot)com>> wrote:

pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com<mailto:pavel(dot)stehule(at)gmail(dot)com>> napsal:

čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com<mailto:pavel(dot)stehule(at)gmail(dot)com>> napsal:

Hi

3) Any way to define CONSTANTs ?
We already talked a bit about this subject and also Gilles Darold introduces it in this mailing-list topic but I'd like to insist on it.
I think it would be nice to have a way to say that a variable should not be changed once defined.
Maybe it's hard to implement and can be implemented later, but I just want to know if this concern is open.

I played little bit with it and I didn't find any nice solution, but maybe I found the solution. I had ideas about some variants, but almost all time I had a problem with parser's shifts because all potential keywords are not reserved.

last variant, but maybe best is using keyword WITH

So the syntax can looks like

CREATE [ TEMP ] VARIABLE varname [ AS ] type [ NOT NULL ] [ DEFAULT expression ] [ WITH [ OPTIONS ] '(' ... ')' ] ]

What do you think about this syntax? It doesn't need any new keyword, and it easy to enhance it.

CREATE VARIABLE foo AS int DEFAULT 10 WITH OPTIONS ( CONSTANT);

After some more thinking and because in other patch I support syntax CREATE TRANSACTION VARIABLE ... I change my opinion and implemented support for
syntax CREATE IMMUTABLE VARIABLE for define constants.

second try to fix pg_dump

Regards

Pavel

See attached patch

Regards

Pavel

?

Regards

Pavel

Hi Pavel,

I have been reviewing the latest patch (schema-variables-20200229.patch.gz)
and here are few comments:

1- There is a compilation error, when compiled with --with-llvm enabled on
CentOS 7.

llvmjit_expr.c: In function ‘llvm_compile_expr’:
llvmjit_expr.c:1090:5: warning: initialization from incompatible pointer type [enabled by default]
build_EvalXFunc(b, mod, "ExecEvalParamVariable",
^
llvmjit_expr.c:1090:5: warning: (near initialization for ‘(anonymous)[0]’) [enabled by default]
llvmjit_expr.c:1090:5: warning: initialization from incompatible pointer type [enabled by default]
llvmjit_expr.c:1090:5: warning: (near initialization for ‘(anonymous)[0]’) [enabled by default]
llvmjit_expr.c:1090:5: warning: initialization from incompatible pointer type [enabled by default]
llvmjit_expr.c:1090:5: warning: (near initialization for ‘(anonymous)[0]’) [enabled by default]
llvmjit_expr.c:1090:5: warning: passing argument 5 of ‘build_EvalXFuncInt’ from incompatible pointer type [enabled by default]
llvmjit_expr.c:60:21: note: expected ‘struct ExprEvalStep *’ but argument is of type ‘LLVMValueRef’
static LLVMValueRef build_EvalXFuncInt(LLVMBuilderRef b, LLVMModuleRef mod,
^
llvmjit_expr.c:1092:29: error: ‘i’ undeclared (first use in this function)
LLVMBuildBr(b, opblocks[i + 1]);
^
llvmjit_expr.c:1092:29: note: each undeclared identifier is reported only once for each function it appears in
make[2]: *** [llvmjit_expr.o] Error 1

After looking into it, it turns out that:
- parameter order was incorrect in build_EvalXFunc()
- LLVMBuildBr() is using the undeclared variable 'i' whereas it should be
using 'opno'.

2- Similarly, If the default expression is referencing a function or object,
dependency should be marked, so if the function is not dropped silently.
otherwise, a cache lookup error will come.

postgres=# create or replace function foofunc() returns timestamp as $$ begin return now(); end; $$ language plpgsql;
CREATE FUNCTION
postgres=# create schema test;
CREATE SCHEMA
postgres=# create variable test.v1 as timestamp default foofunc();
CREATE VARIABLE
postgres=# drop function foofunc();
DROP FUNCTION
postgres=# select test.v1;
ERROR: cache lookup failed for function 16437

Thank you for this analyze and patches. I merged them to attached patch

3- Variable DEFAULT expression is apparently being evaluated at the time of
first access. whereas I think that It should be at the time of variable
creation. consider the following example:

postgres=# create variable test.v2 as timestamp default now();
CREATE VARIABLE
postgres=# select now();
now
-------------------------------
2020-03-05 12:13:29.775373+00
(1 row)
postgres=# select test.v2;
v2
----------------------------
2020-03-05 12:13:37.192317 -- I was expecting this to be earlier than the above timestamp.
(1 row)

postgres=# select test.v2;
v2
----------------------------
2020-03-05 12:13:37.192317
(1 row)
postgres=# let test.v2 = default;
LET
postgres=# select test.v2;
v2
----------------------------
2020-03-05 12:14:07.538615
(1 row)

This is expected and wanted - same behave has plpgsql variables.

CREATE OR REPLACE FUNCTION public.foo()
RETURNS void
LANGUAGE plpgsql
AS $function$
declare x timestamp default current_timestamp;
begin
raise notice '%', x;
end;
$function$

postgres=# select foo();
NOTICE: 2020-03-05 18:49:12.465054
┌─────┐
│ foo │
╞═════╡
│ │
└─────┘
(1 row)

postgres=# select foo();
NOTICE: 2020-03-05 18:49:13.255197
┌─────┐
│ foo │
╞═════╡
│ │
└─────┘
(1 row)

You can use

CREATE VARIABLE cuser AS text DEFAULT session_user;

Has not any sense to use a value from creating time

And a analogy with CREATE TABLE

CREATE TABLE fooo(a timestamp DEFAULT current_timestamp) -- there is not a create time timestamp

I fixed buggy behave of IMMUTABLE variables

Regards

Pavel

To continue my testing of the patch I made few fixes for the above-mentioned
comments. The patch for those changes is attached if it could be of any use.

--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca<http://www.highgo.ca>