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.stehule@gmail.com]
Envoyé : jeudi 5 mars 2020 18:54
À : Asif Rehman <asifr.rehman@gmail.com>
Cc : DUVAL REMI <REMI.DUVAL@CHEOPS.FR>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Objet : Re: proposal: schema variables

 

 

 

čt 5. 3. 2020 v 15:11 odesílatel Asif Rehman <asifr.rehman@gmail.com> napsal:

 

 

On Sat, Feb 29, 2020 at 2:10 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:

 

 

pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule <pavel.stehule@gmail.com> napsal:

 

 

čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule <pavel.stehule@gmail.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