public inbox for pgsql-performance@postgresql.org
help / color / mirror / Atom feedFrom: Pavel Stehule <pavel.stehule@gmail.com>
To: DUVAL REMI <REMI.DUVAL@cheops.fr>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: proposal: schema variables
Date: Fri, 6 Mar 2020 19:54:35 +0100
Message-ID: <CAFj8pRDaGtWaTY5PD4Hp+yuzxhuseqq=N3HMVv0hAza2srKEvw@mail.gmail.com> (raw)
In-Reply-To: <158cad51e4004c70ab621af77edae2ee@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>
References: <CAFj8pRDY+m9OOxfO10R7J0PAkCCauM-TweaTrdsrsLGMb1VbEQ@mail.gmail.com>
<CAFj8pRBQSbOvK94QJCZGY7xqiuT7R7tuGKgci+CXGYONs5PGOw@mail.gmail.com>
<158272883575.1651.2316685587547965508.pgcf@coridan.postgresql.org>
<CAFj8pRDj7ss7YNRdF1nD6=DaeAk7YJ=Nz6449-g0DrPEZmxEug@mail.gmail.com>
<CAFj8pRC9H8s3=Twpk4e+As6Sz7NrceynEKsupg5JLNpVBe4sqQ@mail.gmail.com>
<CAFj8pRDZ-SxOwfupyD=YhGAMKjNetWhGVkdq9RUs-44wjpmxPQ@mail.gmail.com>
<CAFj8pRALAZOu6bXTaz6CHox9S=OqHT-suX+LRgA4z0nzhXv4Vw@mail.gmail.com>
<CADM=Jej3onf9VK_3BfsuCpRLnXrYKp+cCY2PtahpCXRY4jG1iw@mail.gmail.com>
<CAFj8pRCzjcYXTOgOoM41RwVr38mPf172_A184pPLeybA_QvqpQ@mail.gmail.com>
<158cad51e4004c70ab621af77edae2ee@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>
pá 6. 3. 2020 v 16:44 odesílatel DUVAL REMI <REMI.DUVAL@cheops.fr> napsal:
> 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)
>
fixed
- 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.
>
fixed
> - 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).
>
probably it should be somewhere
https://www.postgresql.org/docs/current/plpgsql-porting.html
I wrote note there
> - 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
>
This sentence is used more times (from alter_view0
To alter the owner, you must also be a direct or indirect member of the new
owning role, and that role must have <literal>CREATE</literal> privilege
on
the view's schema. (These restrictions enforce that altering the owner
doesn't do anything you couldn't do by dropping and recreating the view.
However, a superuser can alter ownership of any view anyway.)
>
>
> Otherwise, this is a really nice feature and I’m looking forward to have
> it in PostgreSQL.
>
Thank you very much
updated patch attached
>
> 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
>
>
Attachments:
[text/x-patch] doc.patch (2.4K, 3-doc.patch)
download | inline diff:
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 2987a555a3..8c9bd28aa9 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -5782,6 +5782,19 @@ $$ LANGUAGE plpgsql STRICT IMMUTABLE;
</programlisting>
</para>
</sect3>
+
+ <sect3>
+ <title><command>Global variables and constants</command></title>
+
+ <para>
+ The <application>PL/pgSQL</application> language has not packages
+ and then it has not package variables and package constants. The
+ <productname>PostgreSQL</productname> has schema variables and
+ immutable schema variables. The schema variables can be created
+ by <command>CREATE VARIABLE</command> described in <xref
+ linkend="sql-createvariable"/>.
+ </para>
+ </sect3>
</sect2>
<sect2 id="plpgsql-porting-appendix">
diff --git a/doc/src/sgml/ref/create_variable.sgml b/doc/src/sgml/ref/create_variable.sgml
index 35a14c7c3c..1b1883a11a 100644
--- a/doc/src/sgml/ref/create_variable.sgml
+++ b/doc/src/sgml/ref/create_variable.sgml
@@ -58,7 +58,7 @@ CREATE [ { TEMPORARY | TEMP } ] [ IMMUTABLE ] VARIABLE [ IF NOT EXISTS ] <replac
<term><literal>IMMUTABLE</literal></term>
<listitem>
<para>
- The value of variable cannot be changed.
+ The value of the variable cannot be changed.
</para>
</listitem>
</varlistentry>
@@ -128,13 +128,12 @@ CREATE [ { TEMPORARY | TEMP } ] [ IMMUTABLE ] VARIABLE [ IF NOT EXISTS ] <replac
<term><literal>ON COMMIT DROP</literal>, <literal>ON TRANSACTIONAL END RESET</literal></term>
<listitem>
<para>
- The <literal>ON COMMIT DROP</literal> clause specifies the bahaviour of
- temporary schema variable at transaction commit. With this clause the
+ The <literal>ON COMMIT DROP</literal> clause specifies the behaviour
+ of temporary schema variable at transaction commit. With this clause the
variable is dropped at commit time. The clause is only allowed
- for temporary variables.
- The <literal>ON TRANSACTIONAL END RESET</literal> clause enforces the
- variable to be reset to its default value when the transaction is either
- commited or rolled back.
+ for temporary variables. The <literal>ON TRANSACTIONAL END RESET</literal>
+ clause enforces the variable to be reset to its default value when
+ the transaction is either commited or rolled back.
</para>
</listitem>
</varlistentry>
[application/gzip] schema-variables-20200306.patch.gz (65.2K, 4-schema-variables-20200306.patch.gz)
download
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-performance@postgresql.org
Cc: pavel.stehule@gmail.com, REMI.DUVAL@cheops.fr, pgsql-hackers@lists.postgresql.org
Subject: Re: proposal: schema variables
In-Reply-To: <CAFj8pRDaGtWaTY5PD4Hp+yuzxhuseqq=N3HMVv0hAza2srKEvw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox