public inbox for pgsql-performance@postgresql.org
help / color / mirror / Atom feedFrom: Asif Rehman <asifr.rehman@gmail.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: remi duval <remi.duval@cheops.fr>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: proposal: schema variables
Date: Thu, 5 Mar 2020 19:10:49 +0500
Message-ID: <CADM=Jej3onf9VK_3BfsuCpRLnXrYKp+cCY2PtahpCXRY4jG1iw@mail.gmail.com> (raw)
In-Reply-To: <CAFj8pRALAZOu6bXTaz6CHox9S=OqHT-suX+LRgA4z0nzhXv4Vw@mail.gmail.com>
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>
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
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)
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:
[application/octet-stream] sv-fixes.patch (1.1K, 3-sv-fixes.patch)
download | inline diff:
diff --git a/src/backend/catalog/pg_variable.c b/src/backend/catalog/pg_variable.c
index f32d049bd59..bdee121cb57 100644
--- a/src/backend/catalog/pg_variable.c
+++ b/src/backend/catalog/pg_variable.c
@@ -350,6 +350,11 @@ VariableCreate(const char *varName,
referenced.objectSubId = 0;
recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+ /* dependency on default expr */
+ if (varDefexpr)
+ recordDependencyOnExpr(&myself, (Node *) varDefexpr,
+ NIL, DEPENDENCY_NORMAL);
+
/* dependency on any roles mentioned in ACL */
if (varacl != NULL)
{
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index dde6a5acbdb..08cf7e63bbf 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1088,8 +1088,8 @@ llvm_compile_expr(ExprState *state)
case EEOP_PARAM_VARIABLE:
build_EvalXFunc(b, mod, "ExecEvalParamVariable",
- v_state, v_econtext, op);
- LLVMBuildBr(b, opblocks[i + 1]);
+ v_state, op, v_econtext);
+ LLVMBuildBr(b, opblocks[opno + 1]);
break;
case EEOP_PARAM_CALLBACK:
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: asifr.rehman@gmail.com, pavel.stehule@gmail.com, remi.duval@cheops.fr, pgsql-hackers@lists.postgresql.org
Subject: Re: proposal: schema variables
In-Reply-To: <CADM=Jej3onf9VK_3BfsuCpRLnXrYKp+cCY2PtahpCXRY4jG1iw@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