pá 6. 3. 2020 v 16:44 odesílatel DUVAL REMI <remi.du...@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.steh...@gmail.com]
> *Envoyé :* jeudi 5 mars 2020 18:54
> *À :* Asif Rehman <asifr.reh...@gmail.com>
> *Cc :* DUVAL REMI <remi.du...@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.reh...@gmail.com>
> napsal:
>
>
>
>
>
> On Sat, Feb 29, 2020 at 2:10 PM Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
>
>
>
>
>
> pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule <pavel.steh...@gmail.com>
> napsal:
>
>
>
>
>
> čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule <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: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
>
>
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>

Attachment: schema-variables-20200306.patch.gz
Description: application/gzip

Reply via email to