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 13).
NB: the condition is “if internal_version < 13 => 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.steh...@gmail.com]
Envoyé : jeudi 5 mars 2020 18:54
À : Asif Rehman
Cc : DUVAL REMI ; PostgreSQL Hackers
Objet : Re: proposal: schema variables
čt 5. 3. 2020 v 15:11 odesílatel Asif Rehman
mailto:asifr.reh...@gmail.com>> napsal:
On Sat, Feb 29, 2020 at 2:10 PM Pavel Stehule
mailto:pavel.steh...@gmail.com>> wrote:
pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule
mailto:pavel.steh...@gmail.com>> napsal:
čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule
mailto:pavel.steh...@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:
