Re: proposal: schema variables
Hi I am sending a strongly updated patch for schema variables. I rewrote an execution of a LET statement. In the previous implementation I hacked STMT_SELECT. Now, I introduced a new statement STMT_LET, and I implemented a new executor node SetVariable. Now I think this implementation is much cleaner. Implementation with own executor node reduces necessary work on PL side - and allows the LET statement to be prepared - what is important from a security view. I'll try to write a second implementation based on a cleaner implementation like utility command too. I expect so this version will be more simple, but utility commands cannot be prepared, and probably, there should be special support for any PL. I hope a cleaner implementation can help to move this patch. We can choose one variant in the next step and this variant can be finalized. Notes, comments? Regards Pavel schema-variables-rev2-20210410.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi so 13. 3. 2021 v 7:01 odesílatel Pavel Stehule napsal: > Hi > > fresh rebase > only rebase Regards Pavel > > Pavel > schema-variables-20210404.patch.gz Description: application/gzip
Re: proposal: schema variables - doc
Hi
po 22. 3. 2021 v 10:47 odesílatel Pavel Stehule
napsal:
> Hi
>
> st 17. 3. 2021 v 13:05 odesílatel Erik Rijkers napsal:
>
>>
>> > On 2021.03.13. 07:01 Pavel Stehule wrote:
>> > Hi
>> > fresh rebase
>> > [schema-variables-20210313.patch.gz]
>>
>>
>> Hi Pavel,
>>
>> I notice that the phrase 'schema variable' is not in the index at the end
>> ('bookindex.html'). Not good.
>>
>> It is also not in the index at the front of the manual - also not good.
>>
>> Maybe these two (front and back index) can be added?
>>
>
> I inserted new indexterm "schema variable", and now this part of
> bookindex.html looks like:
>
> schema variablealtering, ALTER VARIABLEchanging, LETdefining, CREATE
> VARIABLEdescription, Descriptionremoving, DROP VARIABLE
>
>
>
>>
>> If a user searches the pdf, the first occurrence he finds is at:
>>
>> 43.13.2.4. Global variables and constants
>> (in itself that occurrence/mention is all right, but is should not be
>> the first find, I think)
>>
>> (I think there was in earlier versions of the patch an entry in the
>> 'contents', i.e., at the front of the manual). I think it would be good to
>> have it in the front-index, pointing to either LET or CREATE VARIABLE, or
>> maybe even to a small introductory paragraph somewhere else (again, I seem
>> to remember that there was one in an earlier patch version).
>>
>
>
> I wrote new section to "advanced features" about schema variables
>
>
>>
>>
>> Of the new commands that this patch brings, 'LET' is the most immediately
>> illuminating for a user (even when a CREATE VARIABLE has to be done first.
>> There is an entry 'LET' in the index (good), but it would be better if that
>> with LET-entry too the phrase 'schema variable' occurred. (I don't know if
>> that's possible)
>>
>>
>> Then, in the CREATE VARIABLE paragraphs it says
>>'Changing a schema variable is non-transactional by default.'
>>
>> I think that, unless there exists a mode where schema vars can be made
>> transactional, 'by default' should be deleted (and there is no such
>> 'transactional mode' for schema variables, is there?). The 'Description'
>> also has such a 'By default' which is better removed for the same reason.
>>
>
> fixed
>
>
>>
>> In the CREATE VARIABLE page the example is:
>>
>> CREATE VARIABLE var1 AS integer;
>> SELECT var1;
>>
>> I suggest to make that
>>
>> CREATE VARIABLE var1 AS date;
>> LET var1 = (select current_date);
>> SELECT var1;
>>
>> So that the example immediately shows an application of functionality.
>>
>
> done
>
> Thank you for the documentation review.
>
> Updated patch attached
>
> Regards
>
> Pavel
>
>
fresh update with merged Eric's changes in documentation
Regards
Pavel
>
>>
>> Thanks,
>>
>> Erik Rijkers
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> >
>> > Pavel
>>
>
schema-variables-20210325.patch.gz
Description: application/gzip
Re: proposal: schema variables - doc
Hi
st 17. 3. 2021 v 13:05 odesílatel Erik Rijkers napsal:
>
> > On 2021.03.13. 07:01 Pavel Stehule wrote:
> > Hi
> > fresh rebase
> > [schema-variables-20210313.patch.gz]
>
>
> Hi Pavel,
>
> I notice that the phrase 'schema variable' is not in the index at the end
> ('bookindex.html'). Not good.
>
> It is also not in the index at the front of the manual - also not good.
>
> Maybe these two (front and back index) can be added?
>
I inserted new indexterm "schema variable", and now this part of
bookindex.html looks like:
schema variablealtering, ALTER VARIABLEchanging, LETdefining, CREATE
VARIABLEdescription, Descriptionremoving, DROP VARIABLE
>
> If a user searches the pdf, the first occurrence he finds is at:
>
> 43.13.2.4. Global variables and constants
> (in itself that occurrence/mention is all right, but is should not be
> the first find, I think)
>
> (I think there was in earlier versions of the patch an entry in the
> 'contents', i.e., at the front of the manual). I think it would be good to
> have it in the front-index, pointing to either LET or CREATE VARIABLE, or
> maybe even to a small introductory paragraph somewhere else (again, I seem
> to remember that there was one in an earlier patch version).
>
I wrote new section to "advanced features" about schema variables
>
>
> Of the new commands that this patch brings, 'LET' is the most immediately
> illuminating for a user (even when a CREATE VARIABLE has to be done first.
> There is an entry 'LET' in the index (good), but it would be better if that
> with LET-entry too the phrase 'schema variable' occurred. (I don't know if
> that's possible)
>
>
> Then, in the CREATE VARIABLE paragraphs it says
>'Changing a schema variable is non-transactional by default.'
>
> I think that, unless there exists a mode where schema vars can be made
> transactional, 'by default' should be deleted (and there is no such
> 'transactional mode' for schema variables, is there?). The 'Description'
> also has such a 'By default' which is better removed for the same reason.
>
fixed
>
> In the CREATE VARIABLE page the example is:
>
> CREATE VARIABLE var1 AS integer;
> SELECT var1;
>
> I suggest to make that
>
> CREATE VARIABLE var1 AS date;
> LET var1 = (select current_date);
> SELECT var1;
>
> So that the example immediately shows an application of functionality.
>
done
Thank you for the documentation review.
Updated patch attached
Regards
Pavel
>
> Thanks,
>
> Erik Rijkers
>
>
>
>
>
>
>
>
>
>
>
>
>
> >
> > Pavel
>
schema-variables-20210322.patch.gz
Description: application/gzip
Re: proposal: schema variables - doc
> On 2021.03.13. 07:01 Pavel Stehule wrote:
> Hi
> fresh rebase
> [schema-variables-20210313.patch.gz]
Hi Pavel,
I notice that the phrase 'schema variable' is not in the index at the end
('bookindex.html'). Not good.
It is also not in the index at the front of the manual - also not good.
Maybe these two (front and back index) can be added?
If a user searches the pdf, the first occurrence he finds is at:
43.13.2.4. Global variables and constants
(in itself that occurrence/mention is all right, but is should not be the
first find, I think)
(I think there was in earlier versions of the patch an entry in the 'contents',
i.e., at the front of the manual). I think it would be good to have it in the
front-index, pointing to either LET or CREATE VARIABLE, or maybe even to a
small introductory paragraph somewhere else (again, I seem to remember that
there was one in an earlier patch version).
Of the new commands that this patch brings, 'LET' is the most immediately
illuminating for a user (even when a CREATE VARIABLE has to be done first.
There is an entry 'LET' in the index (good), but it would be better if that
with LET-entry too the phrase 'schema variable' occurred. (I don't know if
that's possible)
Then, in the CREATE VARIABLE paragraphs it says
'Changing a schema variable is non-transactional by default.'
I think that, unless there exists a mode where schema vars can be made
transactional, 'by default' should be deleted (and there is no such
'transactional mode' for schema variables, is there?). The 'Description' also
has such a 'By default' which is better removed for the same reason.
In the CREATE VARIABLE page the example is:
CREATE VARIABLE var1 AS integer;
SELECT var1;
I suggest to make that
CREATE VARIABLE var1 AS date;
LET var1 = (select current_date);
SELECT var1;
So that the example immediately shows an application of functionality.
Thanks,
Erik Rijkers
>
> Pavel
Re: proposal: schema variables
Hi fresh rebase Pavel schema-variables-20210313.patch.gz Description: application/gzip
Re: proposal: schema variables
út 16. 2. 2021 v 18:46 odesílatel Pavel Stehule napsal: > Hi > > út 2. 2. 2021 v 9:43 odesílatel Pavel Stehule > napsal: > >> Hi >> >> rebase and set PK for pg_variable table >> > > rebase > rebase > Pavel > > >> Regards >> >> Pavel >> > schema-variables-20200301.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi út 2. 2. 2021 v 9:43 odesílatel Pavel Stehule napsal: > Hi > > rebase and set PK for pg_variable table > rebase Pavel > Regards > > Pavel > schema-variables-20200216.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi rebase and set PK for pg_variable table Regards Pavel schema-variables-20210202.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi only rebase Regards Pavel schema-variables-20200123.patch.gz Description: application/gzip
Re: proposal: schema variables
po 18. 1. 2021 v 15:24 odesílatel Erik Rijkers napsal: > On 2021-01-18 10:59, Pavel Stehule wrote: > >> > > and here is the patch > > [schema-variables-20200118.patch.gz ] > > > One small thing: > > The drop variable synopsis is: > > DROP VARIABLE [ IF EXISTS ] name [, ...] [ CASCADE | RESTRICT ] > > In that text following it, 'RESTRICT' is not documented. When used it > does not give an error but I don't see how it 'works'. > should be fixed now. Thank you for check Regards Pavel > > Erik > > > > schema-variables-20200118-2.patch.gz Description: application/gzip
Re: proposal: schema variables
On 2021-01-18 10:59, Pavel Stehule wrote: and here is the patch [schema-variables-20200118.patch.gz ] One small thing: The drop variable synopsis is: DROP VARIABLE [ IF EXISTS ] name [, ...] [ CASCADE | RESTRICT ] In that text following it, 'RESTRICT' is not documented. When used it does not give an error but I don't see how it 'works'. Erik
Re: proposal: schema variables
po 18. 1. 2021 v 10:50 odesílatel Pavel Stehule napsal: > > > čt 14. 1. 2021 v 10:24 odesílatel Erik Rijkers napsal: > >> On 2021-01-14 07:35, Pavel Stehule wrote: >> > [schema-variables-20210114.patch.gz] >> >> >> Build is fine. My (small) list of tests run OK. >> >> I did notice a few more documentation peculiarities: >> >> >> 'The PostgreSQL has schema variables' should be >> 'PostgreSQL has schema variables' >> >> > fixed > > >> A link to the LET command should be added to the 'See Also' of the >> CREATE VARIABLE, ALTER VARIABLE, and DROP VARIABLE pages. (After all, >> the LET command is the most interesting) >> Similarly, an ALTER VARIABLE link should be added to the 'See Also' >> section of LET. >> > > fixed > > >> >> Somehow, the sgml in the doc files causes too large spacing in the html, >> example: >> I copy from the LET html: >> 'if that is defined. If no explicit' >> (6 spaces between 'defined.' and 'If') >> Can you have a look? Sorry - I can't find the cause. It occurs on a >> few more places in the newly added sgml/html. The unwanted spaces are >> visible also in the pdf. >> > > Should be fixed in the last version. Probably there were some problems > with invisible white chars. > > Thank you for check > > Pavel > > > >> (firefox 78.6.1, debian) >> > and here is the patch Regards Pavel >> >> Thanks, >> >> Erik Rijkers >> >> >> schema-variables-20200118.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi čt 14. 1. 2021 v 11:31 odesílatel Josef Šimánek napsal: > I did some testing locally. All runs smoothly, compiled without warning. > > Later on (once merged) it would be nice to write down a documentation > page for the whole feature as a set next to documented individual > commands. > It took me a few moments to understand how this works. > > I was looking how to create variable with non default value in one > command, but if I understand it correctly, that's not the purpose. > Variable lives in a schema with default value, but you can set it per > session via LET. > > Thus "CREATE VARIABLE" statement should not be usually part of > "classic" queries, but it should be threatened more like TABLE or > other DDL statements. > > On the other side LET is there to use the variable in "classic" queries. > > Does that make sense? Feel free to ping me if any help with > documentation would be needed. I can try to prepare an initial > variables guide once I'll ensure I do understand this feature well. > I invite any help with doc. Maybe there can be page in section advanced features https://www.postgresql.org/docs/current/tutorial-advanced.html Regards Pavel > > PS: Now it is clear to me why it is called "schema variables", not > "session variables". > > čt 14. 1. 2021 v 7:36 odesílatel Pavel Stehule > napsal: > > > > Hi > > > > rebase > > > > Regards > > > > Pavel > > >
Re: proposal: schema variables
čt 14. 1. 2021 v 10:24 odesílatel Erik Rijkers napsal: > On 2021-01-14 07:35, Pavel Stehule wrote: > > [schema-variables-20210114.patch.gz] > > > Build is fine. My (small) list of tests run OK. > > I did notice a few more documentation peculiarities: > > > 'The PostgreSQL has schema variables' should be > 'PostgreSQL has schema variables' > > fixed > A link to the LET command should be added to the 'See Also' of the > CREATE VARIABLE, ALTER VARIABLE, and DROP VARIABLE pages. (After all, > the LET command is the most interesting) > Similarly, an ALTER VARIABLE link should be added to the 'See Also' > section of LET. > fixed > > Somehow, the sgml in the doc files causes too large spacing in the html, > example: > I copy from the LET html: > 'if that is defined. If no explicit' > (6 spaces between 'defined.' and 'If') > Can you have a look? Sorry - I can't find the cause. It occurs on a > few more places in the newly added sgml/html. The unwanted spaces are > visible also in the pdf. > Should be fixed in the last version. Probably there were some problems with invisible white chars. Thank you for check Pavel > (firefox 78.6.1, debian) > > > Thanks, > > Erik Rijkers > > >
Re: proposal: schema variables
I did some testing locally. All runs smoothly, compiled without warning. Later on (once merged) it would be nice to write down a documentation page for the whole feature as a set next to documented individual commands. It took me a few moments to understand how this works. I was looking how to create variable with non default value in one command, but if I understand it correctly, that's not the purpose. Variable lives in a schema with default value, but you can set it per session via LET. Thus "CREATE VARIABLE" statement should not be usually part of "classic" queries, but it should be threatened more like TABLE or other DDL statements. On the other side LET is there to use the variable in "classic" queries. Does that make sense? Feel free to ping me if any help with documentation would be needed. I can try to prepare an initial variables guide once I'll ensure I do understand this feature well. PS: Now it is clear to me why it is called "schema variables", not "session variables". čt 14. 1. 2021 v 7:36 odesílatel Pavel Stehule napsal: > > Hi > > rebase > > Regards > > Pavel >
Re: proposal: schema variables
On 2021-01-14 07:35, Pavel Stehule wrote: [schema-variables-20210114.patch.gz] Build is fine. My (small) list of tests run OK. I did notice a few more documentation peculiarities: 'The PostgreSQL has schema variables' should be 'PostgreSQL has schema variables' A link to the LET command should be added to the 'See Also' of the CREATE VARIABLE, ALTER VARIABLE, and DROP VARIABLE pages. (After all, the LET command is the most interesting) Similarly, an ALTER VARIABLE link should be added to the 'See Also' section of LET. Somehow, the sgml in the doc files causes too large spacing in the html, example: I copy from the LET html: 'if that is defined. If no explicit' (6 spaces between 'defined.' and 'If') Can you have a look? Sorry - I can't find the cause. It occurs on a few more places in the newly added sgml/html. The unwanted spaces are visible also in the pdf. (firefox 78.6.1, debian) Thanks, Erik Rijkers
Re: proposal: schema variables
Hi rebase Regards Pavel schema-variables-20210114.patch.gz Description: application/gzip
Re: proposal: schema variables
pá 8. 1. 2021 v 18:54 odesílatel Erik Rijkers napsal: > On 2021-01-08 07:20, Pavel Stehule wrote: > > Hi > > > > just rebase > > > > [schema-variables-20200108.patch] > > Hey Pavel, > > My gcc 8.3.0 compile says: > (on debian 10/Buster) > > utility.c: In function ‘CreateCommandTag’: > utility.c:2332:8: warning: this statement may fall through > [-Wimplicit-fallthrough=] > tag = CMDTAG_SELECT; > ^~~ > utility.c:2334:3: note: here > case T_LetStmt: > ^~~~ > yes, there was an error from the last rebase. Fixed > > compile, check, check-world, runs without further problem. > > I also changed a few typos/improvements in the documentation, see > attached. > > One thing I wasn't sure of: I have assumed that >ON TRANSACTIONAL END RESET > > should be >ON TRANSACTION END RESET > > and changed it accordingly, please double-check. > It looks well, I merged these changes to patch. Thank you very much for these corectures Updated patch attached Regards Pavel > > Erik Rijkers > schema-variables-20210110.patch.gz Description: application/gzip
Re: proposal: schema variables
On 2021-01-08 07:20, Pavel Stehule wrote: Hi just rebase [schema-variables-20200108.patch] Hey Pavel, My gcc 8.3.0 compile says: (on debian 10/Buster) utility.c: In function ‘CreateCommandTag’: utility.c:2332:8: warning: this statement may fall through [-Wimplicit-fallthrough=] tag = CMDTAG_SELECT; ^~~ utility.c:2334:3: note: here case T_LetStmt: ^~~~ compile, check, check-world, runs without further problem. I also changed a few typos/improvements in the documentation, see attached. One thing I wasn't sure of: I have assumed that ON TRANSACTIONAL END RESET should be ON TRANSACTION END RESET and changed it accordingly, please double-check. Erik Rijkers --- doc/src/sgml/ref/create_variable.sgml.orig 2021-01-08 17:40:20.181823036 +0100 +++ doc/src/sgml/ref/create_variable.sgml 2021-01-08 17:59:46.976127524 +0100 @@ -16,7 +16,7 @@ CREATE VARIABLE - define a new permissioned typed schema variable + define a schema variable @@ -29,24 +29,24 @@ Description - The CREATE VARIABLE command creates a new schema variable. + The CREATE VARIABLE command creates a schema variable. Schema variables, like relations, exist within a schema and their access is controlled via GRANT and REVOKE commands. - Their changes are non-transactional by default. + Changing a schema variable is non-transactional by default. The value of a schema variable is local to the current session. Retrieving a variable's value returns either a NULL or a default value, unless its value is set to something else in the current session with a LET command. By default, - the content of variable is not transactional, alike regular variables in PL languages. + the content of a variable is not transactional. This is the same as in regular variables in PL languages. - Schema variables are retrieved by the regular SELECT SQL command. - Their value can be set with the LET SQL command. - Notably, while schema variables share properties with tables, they cannot be updated - with UPDATE commands. + Schema variables are retrieved by the SELECT SQL command. + Their value is set with the LET SQL command. + While schema variables share properties with tables, their value cannot be updated + with an UPDATE command. @@ -76,7 +76,7 @@ name - The name (optionally schema-qualified) of the variable to create. + The name, optionally schema-qualified, of the variable. @@ -85,7 +85,7 @@ data_type - The name (optionally schema-qualified) of the data type of the variable to be created. + The name, optionally schema-qualified, of the data type of the variable. @@ -105,7 +105,7 @@ NOT NULL - The NOT NULL clause forbid to set the variable to + The NOT NULL clause forbids to set the variable to a null value. A variable created as NOT NULL and without an explicitly declared default value cannot be read until it is initialized by a LET command. This obliges the user to explicitly initialize the variable @@ -118,22 +118,22 @@ DEFAULT default_expr - The DEFAULT clause assigns a default data to - schema variable. + The DEFAULT clause can be used to assign a default value to + a schema variable. -ON COMMIT DROP, ON TRANSACTIONAL END RESET +ON COMMIT DROP, ON TRANSACTION END RESET The ON COMMIT DROP clause specifies the behaviour - of temporary schema variable at transaction commit. With this clause the + of a 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 ON TRANSACTIONAL END RESET +      for temporary variables. The ON TRANSACTION END RESET clause enforces the variable to be reset to its default value when - the transaction is either commited or rolled back. + the transaction is committed or rolled back. @@ -145,7 +145,7 @@ Notes - Use DROP VARIABLE command to remove a variable. + Use the DROP VARIABLE command to remove a variable. --- doc/src/sgml/ref/discard.sgml.orig 2021-01-08 18:02:25.837531779 +0100 +++ doc/src/sgml/ref/discard.sgml 2021-01-08 18:40:09.973630164 +0100 @@ -104,6 +104,7 @@ DISCARD PLANS; DISCARD TEMP; DISCARD SEQUENCES; +DISCARD VARIABLES; --- doc/src/sgml/ref/drop_variable.sgml.orig 2021-01-08 18:05:28.643147771 +0100 +++ doc/src/sgml/ref/drop_variable.sgml 2021-01-08 18:07:17.876113523 +0100 @@ -16,7 +16,7 @@ DROP VARIABLE - removes a schema variable + remove a schema variable @@ -52,7 +52,7 @@ name - The name (optionally schema-qualified) of a schema variable. + The name, optionally schema-qualified, of a schema
Re: proposal: schema variables
Hi just rebase Regards Pavel schema-variables-20200108.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi fresh rebase Regards Pavel schema-variables-20210101.patch.gz Description: application/gzip
Re: proposal: schema variables
so 26. 12. 2020 v 7:18 odesílatel Erik Rijkers napsal: > On 2020-12-26 05:52, Pavel Stehule wrote: > > so 19. 12. 2020 v 7:57 odesílatel Pavel Stehule > > > > napsal: > > [schema-variables-20201222.patch.gz (~] > > > >> Hi > >> > >> only rebase > >> > > > > rebase and comments fixes > > > > Hi Pavel, > > This file is the exact same as the file you sent Tuesday. Is it a > mistake? > It is the same. Unfortunately, it was sent in an isolated thread, and commitfest applications didn't find the latest version. I tried to attach new thread to the commitfest application, but without success.
Re: proposal: schema variables
On 2020-12-26 05:52, Pavel Stehule wrote: so 19. 12. 2020 v 7:57 odesílatel Pavel Stehule napsal: [schema-variables-20201222.patch.gz (~] Hi only rebase rebase and comments fixes Hi Pavel, This file is the exact same as the file you sent Tuesday. Is it a mistake?
Re: proposal: schema variables
so 19. 12. 2020 v 7:57 odesílatel Pavel Stehule napsal: > Hi > > only rebase > rebase and comments fixes Regards Pavel > Regards > > Pavel > schema-variables-20201222.patch.gz Description: application/gzip
Re: proposal: schema variables
ne 20. 12. 2020 v 21:43 odesílatel Zhihong Yu napsal:
> Hi,
> This is continuation of the previous review.
>
> +* We should to use schema variable buffer,
> when
> +* it is available.
>
> 'should use' (no to)
>
fixed
> + /* When buffer of used schema variables loaded from shared memory
> */
>
> A verb seems missing in the above comment.
>
I changed this comment
<--><-->/*
<--><--> * link shared memory with working copy of schema variable's values
<--><--> * used in this query. This access is used by parallel query
executor's
<--><--> * workers.
<--><--> */
> + elog(ERROR, "unexpected non-SELECT command in LET ... SELECT");
>
> Since non-SELECT is mentioned, I wonder if the trailing SELECT can be
> omitted.
>
done
> +* some collision can be solved simply here to reduce errors
> based
> +* on simply existence of some variables. Often error can be
> using
>
> simply occurred twice above - I think one should be enough.
> If you want to keep the second, it should be 'simple'.
>
I rewrote this comment
updated patch attached
Regards
Pavel
> Cheers
>
> On Sun, Dec 20, 2020 at 11:25 AM Zhihong Yu wrote:
>
>> Hi,
>> I took a look at the rebased patch.
>>
>> + varisnotnull
>> + boolean
>> +
>> +
>> + True if the schema variable doesn't allow null value. The default
>> value is false.
>>
>> I wonder whether the field can be named in positive tense: e.g.
>> varallowsnull with default of true.
>>
>> + vareoxaction
>> + n = no action, d = drop the
>> variable,
>> + r = reset the variable to its default value.
>>
>> Looks like there is only one action allowed. I wonder if there is a
>> possibility of having two actions at the same time in the future.
>>
>> + The PL/pgSQL language has not packages
>> + and then it has not package variables and package constants. The
>>
>> 'has not' -> 'has no'
>>
>> + a null value. A variable created as NOT NULL and without an
>> explicitely
>>
>> explicitely -> explicitly
>>
>> + int nnewmembers;
>> + Oid*oldmembers;
>> + Oid*newmembers;
>>
>> I wonder if naming nnewmembers newmembercount would be more readable.
>>
>> For pg_variable_aclcheck:
>>
>> + return ACLCHECK_OK;
>> + else
>> + return ACLCHECK_NO_PRIV;
>>
>> The 'else' can be omitted.
>>
>> + * Ownership check for a schema variables (specified by OID).
>>
>> 'a schema variable' (no s)
>>
>> For NamesFromList():
>>
>> + if (IsA(n, String))
>> + {
>> + result = lappend(result, n);
>> + }
>> + else
>> + break;
>>
>> There would be no more name if current n is not a String ?
>>
>> +* both variants, and returns InvalidOid with not_uniq flag,
>> when
>>
>> 'and return' (no s)
>>
>> + return InvalidOid;
>> + }
>> + else if (OidIsValid(varoid_without_attr))
>>
>> 'else' is not needed (since the if block ends with return).
>>
>> For clean_cache_callback(),
>>
>> + if (hash_search(schemavarhashtab,
>> + (void *) >varid,
>> + HASH_REMOVE,
>> + NULL) == NULL)
>> + elog(DEBUG1, "hash table corrupted");
>>
>> Maybe add more information to the debug, such as var name.
>> Should we come out of the while loop in this scenario ?
>>
>> Cheers
>>
>
schema-variables-20201222.patch.gz
Description: application/gzip
Re: proposal: schema variables
Hi
ne 20. 12. 2020 v 20:24 odesílatel Zhihong Yu napsal:
> Hi,
> I took a look at the rebased patch.
>
> + varisnotnull
> + boolean
> +
> +
> + True if the schema variable doesn't allow null value. The default
> value is false.
>
> I wonder whether the field can be named in positive tense: e.g.
> varallowsnull with default of true.
>
although I prefer positive designed variables, in this case this negative
form is better due consistency with other system tables
postgres=# select table_name, column_name from information_schema.columns
where column_name like '%null';
┌──┬──┐
│ table_name │ column_name │
╞══╪══╡
│ pg_type │ typnotnull │
│ pg_attribute │ attnotnull │
│ pg_variable │ varisnotnull │
└──┴──┘
(3 rows)
> + vareoxaction
> + n = no action, d = drop the
> variable,
> + r = reset the variable to its default value.
>
> Looks like there is only one action allowed. I wonder if there is a
> possibility of having two actions at the same time in the future.
>
Surely not - these three possibilities are supported and tested
CREATE VARIABLE t1 AS int DEFAULT -1 ON TRANSACTION END RESET
CREATE TEMP VARIABLE g AS int ON COMMIT DROP;
>
> + The PL/pgSQL language has not packages
> + and then it has not package variables and package constants. The
>
> 'has not' -> 'has no'
>
fixed
> + a null value. A variable created as NOT NULL and without an
> explicitely
>
> explicitely -> explicitly
>
fixed
> + int nnewmembers;
> + Oid*oldmembers;
> + Oid*newmembers;
>
> I wonder if naming nnewmembers newmembercount would be more readable.
>
It is not bad idea, but nnewmembers is used more times on more places, and
then this refactoring should be done in independent patch
> For pg_variable_aclcheck:
>
> + return ACLCHECK_OK;
> + else
> + return ACLCHECK_NO_PRIV;
>
> The 'else' can be omitted.
>
again - this pattern is used more often in related source file, and I used
it for consistency with other functions. It is not visible from the patch,
but when you check the related file, it will be clean.
> + * Ownership check for a schema variables (specified by OID).
>
> 'a schema variable' (no s)
>
> For NamesFromList():
>
> + if (IsA(n, String))
> + {
> + result = lappend(result, n);
> + }
> + else
> + break;
>
> There would be no more name if current n is not a String ?
>
It tries to derive the name of a variable from the target list. Usually
there can be only strings, but there can be array subscripting too
(A_indices node)
I wrote a comment there.
>
> +* both variants, and returns InvalidOid with not_uniq flag,
> when
>
> 'and return' (no s)
>
> + return InvalidOid;
> + }
> + else if (OidIsValid(varoid_without_attr))
>
> 'else' is not needed (since the if block ends with return).
>
I understand. The `else` is not necessary, but I think so it is better due
symmetry
if ()
{
return ..
}
else if ()
{
return ..
}
else
{
return ..
}
This style is used more times in Postgres, and usually I prefer it, when I
want to emphasize so all ways have some similar pattern. My opinion about
it is not too strong, The functionality is same, and I think so readability
is a little bit better (due symmetry) (but I understand well so this can be
subjective).
> For clean_cache_callback(),
>
> + if (hash_search(schemavarhashtab,
> + (void *) >varid,
> + HASH_REMOVE,
> + NULL) == NULL)
> + elog(DEBUG1, "hash table corrupted");
>
> Maybe add more information to the debug, such as var name.
> Should we come out of the while loop in this scenario ?
>
I checked other places, and the same pattern is used in this text. If there
are problems, then the problem is not with some specific schema variable,
but in implementation of a hash table.
Maybe it is better to ignore the result (I did it), like it is done on some
other places. This part is implementation of some simple garbage collector,
and is not important if value was removed this or different way. I changed
comments for this routine.
Regards
Pavel
> Cheers
>
Re: proposal: schema variables
Hi,
This is continuation of the previous review.
+* We should to use schema variable buffer, when
+* it is available.
'should use' (no to)
+ /* When buffer of used schema variables loaded from shared memory */
A verb seems missing in the above comment.
+ elog(ERROR, "unexpected non-SELECT command in LET ... SELECT");
Since non-SELECT is mentioned, I wonder if the trailing SELECT can be
omitted.
+* some collision can be solved simply here to reduce errors
based
+* on simply existence of some variables. Often error can be
using
simply occurred twice above - I think one should be enough.
If you want to keep the second, it should be 'simple'.
Cheers
On Sun, Dec 20, 2020 at 11:25 AM Zhihong Yu wrote:
> Hi,
> I took a look at the rebased patch.
>
> + varisnotnull
> + boolean
> +
> +
> + True if the schema variable doesn't allow null value. The default
> value is false.
>
> I wonder whether the field can be named in positive tense: e.g.
> varallowsnull with default of true.
>
> + vareoxaction
> + n = no action, d = drop the
> variable,
> + r = reset the variable to its default value.
>
> Looks like there is only one action allowed. I wonder if there is a
> possibility of having two actions at the same time in the future.
>
> + The PL/pgSQL language has not packages
> + and then it has not package variables and package constants. The
>
> 'has not' -> 'has no'
>
> + a null value. A variable created as NOT NULL and without an
> explicitely
>
> explicitely -> explicitly
>
> + int nnewmembers;
> + Oid*oldmembers;
> + Oid*newmembers;
>
> I wonder if naming nnewmembers newmembercount would be more readable.
>
> For pg_variable_aclcheck:
>
> + return ACLCHECK_OK;
> + else
> + return ACLCHECK_NO_PRIV;
>
> The 'else' can be omitted.
>
> + * Ownership check for a schema variables (specified by OID).
>
> 'a schema variable' (no s)
>
> For NamesFromList():
>
> + if (IsA(n, String))
> + {
> + result = lappend(result, n);
> + }
> + else
> + break;
>
> There would be no more name if current n is not a String ?
>
> +* both variants, and returns InvalidOid with not_uniq flag,
> when
>
> 'and return' (no s)
>
> + return InvalidOid;
> + }
> + else if (OidIsValid(varoid_without_attr))
>
> 'else' is not needed (since the if block ends with return).
>
> For clean_cache_callback(),
>
> + if (hash_search(schemavarhashtab,
> + (void *) >varid,
> + HASH_REMOVE,
> + NULL) == NULL)
> + elog(DEBUG1, "hash table corrupted");
>
> Maybe add more information to the debug, such as var name.
> Should we come out of the while loop in this scenario ?
>
> Cheers
>
Re: proposal: schema variables
Hi,
I took a look at the rebased patch.
+ varisnotnull
+ boolean
+
+
+ True if the schema variable doesn't allow null value. The default
value is false.
I wonder whether the field can be named in positive tense: e.g.
varallowsnull with default of true.
+ vareoxaction
+ n = no action, d = drop the
variable,
+ r = reset the variable to its default value.
Looks like there is only one action allowed. I wonder if there is a
possibility of having two actions at the same time in the future.
+ The PL/pgSQL language has not packages
+ and then it has not package variables and package constants. The
'has not' -> 'has no'
+ a null value. A variable created as NOT NULL and without an
explicitely
explicitely -> explicitly
+ int nnewmembers;
+ Oid*oldmembers;
+ Oid*newmembers;
I wonder if naming nnewmembers newmembercount would be more readable.
For pg_variable_aclcheck:
+ return ACLCHECK_OK;
+ else
+ return ACLCHECK_NO_PRIV;
The 'else' can be omitted.
+ * Ownership check for a schema variables (specified by OID).
'a schema variable' (no s)
For NamesFromList():
+ if (IsA(n, String))
+ {
+ result = lappend(result, n);
+ }
+ else
+ break;
There would be no more name if current n is not a String ?
+* both variants, and returns InvalidOid with not_uniq flag,
when
'and return' (no s)
+ return InvalidOid;
+ }
+ else if (OidIsValid(varoid_without_attr))
'else' is not needed (since the if block ends with return).
For clean_cache_callback(),
+ if (hash_search(schemavarhashtab,
+ (void *) >varid,
+ HASH_REMOVE,
+ NULL) == NULL)
+ elog(DEBUG1, "hash table corrupted");
Maybe add more information to the debug, such as var name.
Should we come out of the while loop in this scenario ?
Cheers
Re: proposal: schema variables
Hi only rebase Regards Pavel schema-variables-20201219.patch.gz Description: application/gzip
Re: proposal: schema variables
čt 1. 10. 2020 v 7:08 odesílatel Pavel Stehule napsal: > > > čt 1. 10. 2020 v 5:38 odesílatel Michael Paquier > napsal: > >> On Thu, Sep 24, 2020 at 08:49:50PM +0200, Pavel Stehule wrote: >> > fixed patch attached >> >> It looks like there are again conflicts within setrefs.c. >> > > fresh patch > rebase > Regards > > Pavel > > -- >> Michael >> > schema-variables-20201110.patch.gz Description: application/gzip
Re: proposal: schema variables
čt 1. 10. 2020 v 5:38 odesílatel Michael Paquier napsal: > On Thu, Sep 24, 2020 at 08:49:50PM +0200, Pavel Stehule wrote: > > fixed patch attached > > It looks like there are again conflicts within setrefs.c. > fresh patch Regards Pavel -- > Michael > schema-variables-20201001.patch.gz Description: application/gzip
Re: proposal: schema variables
On Thu, Sep 24, 2020 at 08:49:50PM +0200, Pavel Stehule wrote: > fixed patch attached It looks like there are again conflicts within setrefs.c. -- Michael signature.asc Description: PGP signature
Re: proposal: schema variables
čt 24. 9. 2020 v 5:58 odesílatel Pavel Stehule napsal: > > > čt 24. 9. 2020 v 5:56 odesílatel Michael Paquier > napsal: > >> On Sat, Jul 11, 2020 at 06:44:24AM +0200, Pavel Stehule wrote: >> > rebase >> >> Per the CF bot, this needs an extra rebase as it does not apply >> anymore. This has not attracted much the attention of committers as >> well. >> > > I'll fix it today > fixed patch attached Regards Pavel > -- >> Michael >> > schema-variables-20200924.patch.gz Description: application/gzip
Re: proposal: schema variables
čt 24. 9. 2020 v 5:56 odesílatel Michael Paquier napsal: > On Sat, Jul 11, 2020 at 06:44:24AM +0200, Pavel Stehule wrote: > > rebase > > Per the CF bot, this needs an extra rebase as it does not apply > anymore. This has not attracted much the attention of committers as > well. > I'll fix it today -- > Michael >
Re: proposal: schema variables
On Sat, Jul 11, 2020 at 06:44:24AM +0200, Pavel Stehule wrote: > rebase Per the CF bot, this needs an extra rebase as it does not apply anymore. This has not attracted much the attention of committers as well. -- Michael signature.asc Description: PGP signature
Re: proposal: schema variables
po 6. 7. 2020 v 10:17 odesílatel Pavel Stehule napsal: > > > ne 5. 7. 2020 v 15:33 odesílatel Pavel Stehule > napsal: > >> >> >> čt 21. 5. 2020 v 14:49 odesílatel Pavel Stehule >> napsal: >> >>> >>> >>> čt 21. 5. 2020 v 13:34 odesílatel Amit Kapila >>> napsal: >>> On Thu, May 21, 2020 at 3:41 PM Pavel Stehule wrote: > > Hi > > just rebase without any other changes > You seem to forget attaching the rebased patch. >>> >>> I am sorry >>> >>> attached. >>> >> >> fresh rebase >> > > fix test build > rebase Pavel > Regards > > Pavel > > >> Regards >> >> Pavel >> >> >>> Pavel >>> >>> -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com >>> schema-variables-20200711.patch.gz Description: application/gzip
Re: proposal: schema variables
ne 5. 7. 2020 v 15:33 odesílatel Pavel Stehule napsal: > > > čt 21. 5. 2020 v 14:49 odesílatel Pavel Stehule > napsal: > >> >> >> čt 21. 5. 2020 v 13:34 odesílatel Amit Kapila >> napsal: >> >>> On Thu, May 21, 2020 at 3:41 PM Pavel Stehule >>> wrote: >>> > >>> > Hi >>> > >>> > just rebase without any other changes >>> > >>> >>> You seem to forget attaching the rebased patch. >>> >> >> I am sorry >> >> attached. >> > > fresh rebase > fix test build Regards Pavel > Regards > > Pavel > > >> Pavel >> >> >>> -- >>> With Regards, >>> Amit Kapila. >>> EnterpriseDB: http://www.enterprisedb.com >>> >> schema-variables-20200706.patch.gz Description: application/gzip
Re: proposal: schema variables
čt 21. 5. 2020 v 14:49 odesílatel Pavel Stehule napsal: > > > čt 21. 5. 2020 v 13:34 odesílatel Amit Kapila > napsal: > >> On Thu, May 21, 2020 at 3:41 PM Pavel Stehule >> wrote: >> > >> > Hi >> > >> > just rebase without any other changes >> > >> >> You seem to forget attaching the rebased patch. >> > > I am sorry > > attached. > fresh rebase Regards Pavel > Pavel > > >> -- >> With Regards, >> Amit Kapila. >> EnterpriseDB: http://www.enterprisedb.com >> > schema-variables-20200705.patch.gz Description: application/gzip
Re: proposal: schema variables
čt 21. 5. 2020 v 13:34 odesílatel Amit Kapila napsal: > On Thu, May 21, 2020 at 3:41 PM Pavel Stehule > wrote: > > > > Hi > > > > just rebase without any other changes > > > > You seem to forget attaching the rebased patch. > I am sorry attached. Pavel > -- > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com > schema-variables-20200521.patch.gz Description: application/gzip
Re: proposal: schema variables
On Thu, May 21, 2020 at 3:41 PM Pavel Stehule wrote: > > Hi > > just rebase without any other changes > You seem to forget attaching the rebased patch. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: proposal: schema variables
Hi just rebase without any other changes Regards Pavel
Re: proposal: schema variables
Hi rebase Regards Pavel ne 22. 3. 2020 v 8:40 odesílatel Pavel Stehule napsal: > Hi > > rebase > > Regards > > Pavel > schema-variables-20200310.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi rebase Regards Pavel schema-variables-20200322.patch.gz Description: application/gzip
Re: proposal: schema variables
pá 20. 3. 2020 v 8:18 odesílatel Pavel Stehule
napsal:
>
>
> čt 19. 3. 2020 v 10:43 odesílatel DUVAL REMI
> napsal:
>
>> Hello
>>
>>
>>
>> I played around with the ALTER VARIABLE statement to make sure it’s OK
>> and it seems fine to me.
>>
>>
>>
>> Another last thing before commiting.
>>
>>
>>
>> I agree with Thomas Vondra, that this part might/should be simplified :
>>
>>
>>
>> [ { ON COMMIT DROP | ON { TRANSACTIONAL | TRANSACTION } END RESET } ]
>>
>>
>>
>> I would only allow “ON TRANSACTION END RESET”.
>>
>> I think we don’t need both here.
>>
>> Philippe Beaudoin was indeed talking about the TRANSACTIONAL keyword, but
>> that would have make sense (and I think that’s what he meant) , if you
>> could do something like “CREATE [NON] TRANSACTIONAL VARIABLE …”.
>>
>> But here I don’t think that the ON TRANSACTIONAL END RESET has any sense
>> in English, and it only complicates the syntax.
>>
>>
>>
>> Maybe Thomas Vondra (if it’s him) would be more inclined to commit the
>> patch if it has this more simple syntax has he requested.
>>
>>
>>
>> What do you think ?
>>
>
> I removed TRANSACTIONAL from this clause, see attachement change.diff
>
> Updated patch attached.
>
> I hope it would be the last touch, making it fully ready for a commiter.
>>
>
> Thank you very much for review and testing
>
documentation fix
> Pavel
>
>>
schema-variables-20200320-2.patch.gz
Description: application/gzip
Re: proposal: schema variables
čt 19. 3. 2020 v 10:43 odesílatel DUVAL REMI napsal:
> Hello
>
>
>
> I played around with the ALTER VARIABLE statement to make sure it’s OK and
> it seems fine to me.
>
>
>
> Another last thing before commiting.
>
>
>
> I agree with Thomas Vondra, that this part might/should be simplified :
>
>
>
> [ { ON COMMIT DROP | ON { TRANSACTIONAL | TRANSACTION } END RESET } ]
>
>
>
> I would only allow “ON TRANSACTION END RESET”.
>
> I think we don’t need both here.
>
> Philippe Beaudoin was indeed talking about the TRANSACTIONAL keyword, but
> that would have make sense (and I think that’s what he meant) , if you
> could do something like “CREATE [NON] TRANSACTIONAL VARIABLE …”.
>
> But here I don’t think that the ON TRANSACTIONAL END RESET has any sense
> in English, and it only complicates the syntax.
>
>
>
> Maybe Thomas Vondra (if it’s him) would be more inclined to commit the
> patch if it has this more simple syntax has he requested.
>
>
>
> What do you think ?
>
I removed TRANSACTIONAL from this clause, see attachement change.diff
Updated patch attached.
I hope it would be the last touch, making it fully ready for a commiter.
>
Thank you very much for review and testing
Pavel
>
diff --git a/doc/src/sgml/ref/create_variable.sgml b/doc/src/sgml/ref/create_variable.sgml
index 1b1883a11a..cf175e05c6 100644
--- a/doc/src/sgml/ref/create_variable.sgml
+++ b/doc/src/sgml/ref/create_variable.sgml
@@ -22,7 +22,7 @@ PostgreSQL documentation
CREATE [ { TEMPORARY | TEMP } ] [ IMMUTABLE ] VARIABLE [ IF NOT EXISTS ] name [ AS ] data_type ] [ COLLATE collation ]
-[ NOT NULL ] [ DEFAULT default_expr ] [ { ON COMMIT DROP | ON { TRANSACTIONAL | TRANSACTION } END RESET } ]
+[ NOT NULL ] [ DEFAULT default_expr ] [ { ON COMMIT DROP | ON { TRANSACTION } END RESET } ]
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 744342733d..c135a35d07 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4567,7 +4567,6 @@ OptSchemaVarDefExpr: DEFAULT b_expr { $$ = $2; }
OnEOXActionOption: ON COMMIT DROP { $$ = VARIABLE_EOX_DROP; }
| ON TRANSACTION END_P RESET { $$ = VARIABLE_EOX_RESET; }
- | ON TRANSACTIONAL END_P RESET { $$ = VARIABLE_EOX_RESET; }
| /*EMPTY*/{ $$ = VARIABLE_EOX_NOOP; }
;
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d39bcfe9cf..d26b0efcd9 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5978,7 +5978,7 @@
proname => 'pg_collation_is_visible', procost => '10', provolatile => 's',
prorettype => 'bool', proargtypes => 'oid',
prosrc => 'pg_collation_is_visible' },
-{ oid => '4191', descr => 'is schema variable visible in search path?',
+{ oid => '4142', descr => 'is schema variable visible in search path?',
proname => 'pg_variable_is_visible', procost => '10', provolatile => 's',
prorettype => 'bool', proargtypes => 'oid', prosrc => 'pg_variable_is_visible' },
schema-variables-20200320.patch.gz
Description: application/gzip
Re: proposal: schema variables
Hi fresh patch - rebase and fix issue reported by Remi - broken usage CREATE VARIABLE inside PLpgSQL Regards Pavel schema-variables-20200318.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi ne 8. 3. 2020 v 19:12 odesílatel Pavel Stehule napsal: > > > so 7. 3. 2020 v 22:15 odesílatel Pavel Stehule > napsal: > >> Hi >> >> I fixed the some ugly parts of this patch - now the LET x = DEFAULT, LET >> x = NULL is processed by more usual way. >> Statement LET is doesn't switch between STMT_UTILITY and >> STMT_PLAN_UTILITY like before. It is only STMT_PLAN_UTILITY statement. >> > > I did some cleaning - I mainly replaced CMD_PLAN_UTILITY by CMD_LET > because there is not another similar statement in queue. > another cleaning - I repleaced CMD_LET by CMD_SELECT_UTILITY. This name is more illustrative, and little bit cleaned code. CMD_SELECT_UTILITY is mix of CMD_SELECT and CMD_UTILITY. It allows PREPARE and parametrizations like CMD_SELECT. But execution is like CMD_UTILITY by own utility routine with explicit dest receiver setting. This design doesn't need to introduce new executor and planner nodes (like ModifyTable and ModifyTablePath). Regards Pavel > Regards > > Pavel > > >> Regards >> >> Pavel >> > schema-variables-20200313.patch.gz Description: application/gzip
Re: proposal: schema variables
so 7. 3. 2020 v 22:15 odesílatel Pavel Stehule napsal: > Hi > > I fixed the some ugly parts of this patch - now the LET x = DEFAULT, LET x > = NULL is processed by more usual way. > Statement LET is doesn't switch between STMT_UTILITY and STMT_PLAN_UTILITY > like before. It is only STMT_PLAN_UTILITY statement. > I did some cleaning - I mainly replaced CMD_PLAN_UTILITY by CMD_LET because there is not another similar statement in queue. Regards Pavel > Regards > > Pavel > schema-variables-20200308.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi I fixed the some ugly parts of this patch - now the LET x = DEFAULT, LET x = NULL is processed by more usual way. Statement LET is doesn't switch between STMT_UTILITY and STMT_PLAN_UTILITY like before. It is only STMT_PLAN_UTILITY statement. Regards Pavel schema-variables-20200307.patch.gz Description: application/gzip
Re: proposal: schema variables
pá 6. 3. 2020 v 16:44 odesílatel DUVAL REMI 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 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)
>
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 CREATE 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
> *Cc :* DUVAL REMI ; PostgreSQL Hackers <
> pgsql-hackers@lists.postgresql.org>
> *Objet :* Re: proposal: schema variables
>
>
>
>
>
>
>
> čt 5. 3. 2020 v 15:11 odesílatel Asif Rehman
> napsal:
>
>
>
>
>
> On Sat, Feb 29, 2020 at 2:10 PM Pavel Stehule
> wrote:
>
>
>
>
>
> pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule
> napsal:
>
>
>
>
>
> čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule
> 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
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 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:
Re: proposal: schema variables
čt 5. 3. 2020 v 15:11 odesílatel Asif Rehman
napsal:
>
>
> On Sat, Feb 29, 2020 at 2:10 PM Pavel Stehule
> wrote:
>
>>
>>
>> pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule
>> napsal:
>>
>>>
>>>
>>> čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule
>>> 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
Re: proposal: schema variables
On Sat, Feb 29, 2020 at 2:10 PM Pavel Stehule
wrote:
>
>
> pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule
> napsal:
>
>>
>>
>> čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule
>> 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 :
Re: proposal: schema variables
pá 28. 2. 2020 v 16:30 odesílatel Pavel Stehule
napsal:
>
>
> čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule
> 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
>>
>>
>>
schema-variables-20200229.patch.gz
Description: application/gzip
Re: proposal: schema variables
čt 27. 2. 2020 v 15:37 odesílatel Pavel Stehule
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.
See attached patch
Regards
Pavel
>
> ?
>
> Regards
>
> Pavel
>
>
>
schema-variables-20200228.patch.gz
Description: application/gzip
Re: proposal: schema variables
čt 27. 2. 2020 v 15:59 odesílatel DUVAL REMI napsal:
> Hello Pavel.
>
>
>
> That looks pretty good to me !
>
>
>
> I’m adding Philippe Beaudoin who was also interested in this topic.
>
>
>
> Recap : We were looking for a way to separate variable from constants in
> the “Schema Variables” proposition from Pavel.
>
> Pavel was saying that there are some limitations regarding the keywords we
> can use, as the community don’t want to introduce too much new keywords in
> Postgres SQL (PL/pgSQL is a different list of keywords).
>
> “CONSTANT” is not a keyword in SQL for Now (though it is one in PL/pgSQL).
>
> Pavel’s syntax allow to use it as a keyword in the “WITH OPTIONS” clause
> that is already supported.
>
> … I think it’s a good idea.
>
>
>
> The list of keywords is defined in : postgresql\src\include\parser\kwlist.h
>
>
>
> Pavel, I saw that in DB2, those variables are called “Global Variables”,
> is it something we can consider changing, or do you prefer to keep using
> the “Schema Variable” name ?
>
It is most hard question. Global variables has sense, but when we will use
it in plpgsql, then this name can be little bit confusing. Personally I
prefer "schema variable" although my opinion is not too strong. This name
more signalize so this is more generic, more database related than some
special kind of plpgsql variables. Now, I think so maybe is better to use
schema variables, because there is different syntax then global temp
tables. Variables are global by design. So in this moment I prefer the name
"schema variables". It can be used as global variables in plpgsql, but it
is one case.
Pavel
>
>
>
> *De :* Pavel Stehule [mailto:pavel.steh...@gmail.com]
> *Envoyé :* jeudi 27 février 2020 15:38
> *À :* DUVAL REMI
> *Cc :* PostgreSQL Hackers
> *Objet :* Re: proposal: schema variables
>
>
>
>
>
> 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);
>
>
>
> ?
>
>
>
> Regards
>
>
>
> Pavel
>
>
>
>
>
RE: proposal: schema variables
Hello Pavel.
That looks pretty good to me !
I’m adding Philippe Beaudoin who was also interested in this topic.
Recap : We were looking for a way to separate variable from constants in the
“Schema Variables” proposition from Pavel.
Pavel was saying that there are some limitations regarding the keywords we can
use, as the community don’t want to introduce too much new keywords in Postgres
SQL (PL/pgSQL is a different list of keywords).
“CONSTANT” is not a keyword in SQL for Now (though it is one in PL/pgSQL).
Pavel’s syntax allow to use it as a keyword in the “WITH OPTIONS” clause that
is already supported.
… I think it’s a good idea.
The list of keywords is defined in : postgresql\src\include\parser\kwlist.h
Pavel, I saw that in DB2, those variables are called “Global Variables”, is it
something we can consider changing, or do you prefer to keep using the “Schema
Variable” name ?
De : Pavel Stehule [mailto:pavel.steh...@gmail.com]
Envoyé : jeudi 27 février 2020 15:38
À : DUVAL REMI
Cc : PostgreSQL Hackers
Objet : Re: proposal: schema variables
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);
?
Regards
Pavel
Re: proposal: schema variables
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);
?
Regards
Pavel
Re: proposal: schema variables
st 26. 2. 2020 v 15:54 odesílatel remi duval napsal:
> The following review has been posted through the commitfest application:
> make installcheck-world: not tested
> Implements feature: tested, passed
> Spec compliant: tested, failed
> Documentation:tested, failed
>
> Hello Pavel
>
> First thanks for working on this patch cause it might be really helpful
> for those of us trying to migrate PL code between RDBMs.
>
> I tried your patch for migrating an Oracle package body to PL/pgSQL after
> also testing a solution using set_config and current_setting (which works
> but I'm not really satisfied by this workaround solution).
>
> So I compiled latest postgres sources from github on Linux (redhat 7.7)
> using only your patch number 1 (I did not try the second part of the patch).
>
> For my use-case it's working great, performances are excellent (compared
> to other solution for porting "package variables").
> I did not test all the features involved by the patch (especially ALTER
> variable).
>
ALTER VARIABLE is not implemented yet
> I have some feedback however :
>
> 1) Failure when using pg_dump 13 on a 12.1 database
>
> When exporting a 12.1 database using pg_dump from the latest development
> sources I have an error regarding variables export
>
> [pg12@TST-LINUX-PG-03 ~]$ /opt/postgres12/pg12/bin/pg_dump -h localhost
> -p 5432 -U postgres -f dump_pg12.sql database1
> pg_dump: error: query failed: ERROR: relation "pg_variable" does not exist
> LINE 1: ...og.pg_get_expr(v.vardefexpr,0) as vardefexpr FROM pg_variabl...
> ^
> pg_dump: error: query was: SELECT v.tableoid, v.oid, v.varname,
> v.vareoxaction, v.varnamespace,
> (SELECT rolname FROM pg_catalog.pg_roles WHERE oid = varowner) AS rolname
> , (SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM (SELECT acl, row_n
> FROM
> pg_catalog.unnest(coalesce(v.varacl,pg_catalog.acldefault('V',v.varowner)))
> WITH ORDINALITY AS perm(acl,row_n)
> WHERE NOT EXISTS ( SELECT 1 FROM
> pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault('V',v.varowner)))
> AS init(init_acl)
> WHERE acl = init_acl)) as foo) as varacl, ...:
>
> I think that it should have worked anyway cause the documentation states :
> https://www.postgresql.org/docs/current/upgrading.html
> "It is recommended that you use the pg_dump and pg_dumpall programs from
> the newer version of PostgreSQL, to take advantage of enhancements that
> might have been made in these programs." (that's what I did here)
>
> I think there should be a way to avoid dumping the variable if they don't
> exist, should'nt it ?
>
There was a protection against dump 11, but now it should be Postgres 12.
Fixed.
>
> 2) Displaying the variables + completion
> I created 2 variables using :
> CREATE VARIABLE my_pkg.g_dat_deb varchar(11);
> CREATE VARIABLE my_pkg.g_dat_fin varchar(11);
> When I try to display them, I can only see them when prefixing by the
> schema :
> bdd13=> \dV
> "Did not find any schema variables."
> bdd13=> \dV my_pkg.*
>List of variables
>Schema | Name | Type | Is nullable |
> Default | Owner | Transactional end action
>
> ++---+-+-+---+--
> my_pkg| g_dat_deb | character varying(11) | t | |
> myowner |
> my_pkg| g_dat_fin | character varying(11) | t | |
> myowner |
> (3 rows)
>
it is ok - it depends on SEARCH_PATH value
> bdd13=> \dV my_pkg
> Did not find any schema variable named "my_pck".
> NB : Using this template, functions are returned, maybe variables should
> also be listed ? (here by querying on "my_pkg%")
> cts_get13=> \dV my_p [TAB]
> => completion using [TAB] key is not working
>
> Is this normal that I cannot see all the variables when not specifying any
> schema ?
> Also the completion works for functions, but not for variable.
> That's just some bonus but it might be good to have it.
>
> I think the way variables are listed using \dV should match with \df for
> querying functions
>
fixed
> 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.
>
This topic is open. I tried to play with it. The problem is syntax. When I
try to reproduce syntax from PLpgSQL, then I need to introduce new reserved
keyword. So my initial idea was wrong.
We need to open discussion about implementable syntax. For this moment you
can use a workaround - any schema variable without WRITE right is constant.
Implementation is easy. Design
Re: proposal: schema variables
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: tested, passed
Spec compliant: tested, failed
Documentation:tested, failed
Hello Pavel
First thanks for working on this patch cause it might be really helpful for
those of us trying to migrate PL code between RDBMs.
I tried your patch for migrating an Oracle package body to PL/pgSQL after also
testing a solution using set_config and current_setting (which works but I'm
not really satisfied by this workaround solution).
So I compiled latest postgres sources from github on Linux (redhat 7.7) using
only your patch number 1 (I did not try the second part of the patch).
For my use-case it's working great, performances are excellent (compared to
other solution for porting "package variables").
I did not test all the features involved by the patch (especially ALTER
variable).
I have some feedback however :
1) Failure when using pg_dump 13 on a 12.1 database
When exporting a 12.1 database using pg_dump from the latest development
sources I have an error regarding variables export
[pg12@TST-LINUX-PG-03 ~]$ /opt/postgres12/pg12/bin/pg_dump -h localhost -p 5432
-U postgres -f dump_pg12.sql database1
pg_dump: error: query failed: ERROR: relation "pg_variable" does not exist
LINE 1: ...og.pg_get_expr(v.vardefexpr,0) as vardefexpr FROM pg_variabl...
^
pg_dump: error: query was: SELECT v.tableoid, v.oid, v.varname, v.vareoxaction,
v.varnamespace,
(SELECT rolname FROM pg_catalog.pg_roles WHERE oid = varowner) AS rolname
, (SELECT pg_catalog.array_agg(acl ORDER BY row_n) FROM (SELECT acl, row_n
FROM pg_catalog.unnest(coalesce(v.varacl,pg_catalog.acldefault('V',v.varowner)))
WITH ORDINALITY AS perm(acl,row_n)
WHERE NOT EXISTS ( SELECT 1 FROM
pg_catalog.unnest(coalesce(pip.initprivs,pg_catalog.acldefault('V',v.varowner)))
AS init(init_acl)
WHERE acl = init_acl)) as foo) as varacl, ...:
I think that it should have worked anyway cause the documentation states :
https://www.postgresql.org/docs/current/upgrading.html
"It is recommended that you use the pg_dump and pg_dumpall programs from the
newer version of PostgreSQL, to take advantage of enhancements that might have
been made in these programs." (that's what I did here)
I think there should be a way to avoid dumping the variable if they don't
exist, should'nt it ?
2) Displaying the variables + completion
I created 2 variables using :
CREATE VARIABLE my_pkg.g_dat_deb varchar(11);
CREATE VARIABLE my_pkg.g_dat_fin varchar(11);
When I try to display them, I can only see them when prefixing by the schema :
bdd13=> \dV
"Did not find any schema variables."
bdd13=> \dV my_pkg.*
List of variables
Schema | Name | Type | Is nullable | Default |
Owner | Transactional end action
++---+-+-+---+--
my_pkg| g_dat_deb | character varying(11) | t | | myowner
|
my_pkg| g_dat_fin | character varying(11) | t | |
myowner |
(3 rows)
bdd13=> \dV my_pkg
Did not find any schema variable named "my_pck".
NB : Using this template, functions are returned, maybe variables should also
be listed ? (here by querying on "my_pkg%")
cts_get13=> \dV my_p [TAB]
=> completion using [TAB] key is not working
Is this normal that I cannot see all the variables when not specifying any
schema ?
Also the completion works for functions, but not for variable.
That's just some bonus but it might be good to have it.
I think the way variables are listed using \dV should match with \df for
querying functions
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.
Otherwise the documentation looks good to me.
Regards
Rémi
Re: proposal: schema variables
po 10. 2. 2020 v 19:47 odesílatel Pavel Stehule napsal: > > > pá 7. 2. 2020 v 17:09 odesílatel Pavel Stehule > napsal: > >> Hi >> >> rebase >> >> Regards >> >> Pavel >> > > Hi > > another rebase, fix \dV statement (for 0001 patch) > rebase Pavel > Regards > > Pavel > 0001-schema-variables-20200215.patch.gz Description: application/gzip
Re: proposal: schema variables
pá 7. 2. 2020 v 17:09 odesílatel Pavel Stehule napsal: > Hi > > rebase > > Regards > > Pavel > Hi another rebase, fix \dV statement (for 0001 patch) Regards Pavel 0002-transactional-variables-20200210.patch.gz Description: application/gzip 0001-schema-variables-20200210.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi rebase Regards Pavel 0002-transactional-variables-20200207.patch.gz Description: application/gzip 0001-schema-variables-20200207.patch.gz Description: application/gzip
Re: proposal: schema variables
>
>
> 12) I find it rather suspicious that we make decisions in utility.c
> solely based on commandType (whether it's CMD_UTILITY or not). IMO
> it's pretty strange/ugly that T_LetStmt can be both CMD_UTILITY and
> CMD_PLAN_UTILITY:
>
> case T_LetStmt:
> {
> if (pstmt->commandType == CMD_UTILITY)
> doLetStmtReset(pstmt);
> else
> {
> Assert(pstmt->commandType == CMD_PLAN_UTILITY);
> doLetStmtEval(pstmt, params, queryEnv, queryString);
> }
>
> if (completionTag)
> strcpy(completionTag, "LET");
> }
> break;
>
>
>
It looks strange, but it has sense, because the LET stmt supports reset to
default value.
I can write
1. LET var = DEFAULT;
2. LET var = (query);
In first case I have not any query, that I can assign, and in this case the
LET statement is really only UTILITY.
I did comment there
Regards
Pavel
>
>
> regards
>
> --
> Tomas Vondra http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
0001-schema-variables-20200126.patch.gz
Description: application/gzip
Re: proposal: schema variables
st 22. 1. 2020 v 0:41 odesílatel Tomas Vondra
napsal:
> Hi,
>
> I did a quick review of this patch today, so let me share a couple of
> comments.
>
> Firstly, the patch is pretty large - it has ~270kB. Not the largest
> patch ever, but large. I think breaking the patch into smaller pieces
> would significantly improve the chnce of it getting committed.
>
> Is it possible to break the patch into smaller pieces that could be
> applied incrementally? For example, we could move the "transactional"
> behavior into a separate patch, but it's not clear to me how much code
> would that actually move to that second patch. Any other parts that
> could be moved to a separate patch?
>
I am sending two patches - 0001 - schema variables, 0002 - transactional
variables
>
> I see one of the main contention points was a rather long discussion
> about transactional vs. non-transactional behavior. I agree with Pavel's
> position that the non-transactional behavior should be the default,
> simply because it's better aligned with what the other databases are
> doing (and supporting migrations seems like one of the main use cases
> for this feature).
>
> I do understand it may not be suitable for some other use cases,
> mentioned by Fabien, but IMHO it's fine to require explicit
> specification of transactional behavior. Well, we can't have both as
> default, and I don't think there's an obvious reason why it should be
> the other way around.
>
> Now, a bunch of comments about the code (some of that nitpicking):
>
>
> 1) This hunk in doc/src/sgml/catalogs.sgml still talks about "table
> creation" instead of schema creation:
>
>
>vartypmod
>int4
>
>
> vartypmod records type-specific data
> supplied at table creation time (for example, the maximum
> length of a varchar column). It is passed to
> type-specific input functions and length coercion functions.
> The value will generally be -1 for types that do not need
> vartypmod.
>
>
>
fixed
>
> 2) This hunk in doc/src/sgml/ref/alter_default_privileges.sgml uses
> "role_name" instead of "variable_name"
>
> GRANT { READ | WRITE | ALL [ PRIVILEGES ] }
> ON VARIABLES
> TO { [ GROUP ] role_name
> | PUBLIC } [, ...] [ WITH GRANT OPTION ]
>
I think so this is correct
>
> 3) I find the syntax in create_variable.sgml a bit too over-complicated:
>
>
> CREATE [ { TEMPORARY | TEMP } ] [ { TRANSACTIONAL | TRANSACTION } ]
> VARIABLE [ IF NOT EXISTS ] class="parameter">name [ AS ] class="parameter">data_type ] [ COLLATE class="parameter">collation ]
> [ NOT NULL ] [ DEFAULT class="parameter">default_expr ] [ { ON COMMIT DROP | ON {
> TRANSACTIONAL | TRANSACTION } END RESET } ]
>
>
> Do we really need both TRANSACTION and TRANSACTIONAL? Why not just one
> that we already have in the grammar (i.e. TRANSACTION)?
>
It was a Philippe's wish - the implementation is simple, and it is similar
like TEMP, TEMPORARY. I have not any opinion about it.
>
> 4) I think we should rename schemavariable.h to schema_variable.h.
>
done
>
> 5) objectaddress.c has extra line after 'break;' in one switch.
>
fixed
>
> 6) The comment is wrong:
>
> /*
> * Find the ObjectAddress for a type or domain
> */
> static ObjectAddress
> get_object_address_variable(List *object, bool missing_ok)
>
fixed
>
> 7) I think the code/comments are really inconsistent in talking about
> "variables" and "schema variables". For example in objectaddress.c we do
> these two things:
>
> case OCLASS_VARIABLE:
> appendStringInfoString(, "schema variable");
> break;
>
> vs.
>
> case DEFACLOBJ_VARIABLE:
> appendStringInfoString(,
> " on variables");
> break;
>
> That's going to be confusing for people.
>
>
fixed
>
> 8) I'm rather confused by CMD_PLAN_UTILITY, which seems to be defined
> merely to support LET. I'm not sure why that's necessary (Why wouldn't
> CMD_UTILITY be sufficient?).
>
Currently out utility statements cannot to hold a execution plan, and
cannot be prepared.
so this enhancing is motivated mainly by performance reasons. I would to
allow any SELECT query there, not just expressions only (see a limits of
CALL statement)
> Having to add conditions checking for CMD_PLAN_UTILITY to various places
> in planner.c is rather annoying, and I wonder how likely it's this will
> unnecessarily break external code in extensions etc.
>
>
> 9) This comment in planner.c seems obsolete (not updated to reflect
> addition of the CMD_PLAN_UTILITY check):
>
>/*
> * If this is an INSERT/UPDATE/DELETE, and we're not being called from
> * inheritance_planner, add the ModifyTable node.
> */
>if (parse->commandType != CMD_SELECT && parse->commandType !=
> CMD_PLAN_UTILITY && !inheritance_update)
>
"If this is an INSERT/UPDATE/DELETE," is related to
Re: proposal: schema variables
Hi,
I did a quick review of this patch today, so let me share a couple of
comments.
Firstly, the patch is pretty large - it has ~270kB. Not the largest
patch ever, but large. I think breaking the patch into smaller pieces
would significantly improve the chnce of it getting committed.
Is it possible to break the patch into smaller pieces that could be
applied incrementally? For example, we could move the "transactional"
behavior into a separate patch, but it's not clear to me how much code
would that actually move to that second patch. Any other parts that
could be moved to a separate patch?
I see one of the main contention points was a rather long discussion
about transactional vs. non-transactional behavior. I agree with Pavel's
position that the non-transactional behavior should be the default,
simply because it's better aligned with what the other databases are
doing (and supporting migrations seems like one of the main use cases
for this feature).
I do understand it may not be suitable for some other use cases,
mentioned by Fabien, but IMHO it's fine to require explicit
specification of transactional behavior. Well, we can't have both as
default, and I don't think there's an obvious reason why it should be
the other way around.
Now, a bunch of comments about the code (some of that nitpicking):
1) This hunk in doc/src/sgml/catalogs.sgml still talks about "table
creation" instead of schema creation:
vartypmod
int4
vartypmod records type-specific data
supplied at table creation time (for example, the maximum
length of a varchar column). It is passed to
type-specific input functions and length coercion functions.
The value will generally be -1 for types that do not need
vartypmod.
2) This hunk in doc/src/sgml/ref/alter_default_privileges.sgml uses
"role_name" instead of "variable_name"
GRANT { READ | WRITE | ALL [ PRIVILEGES ] }
ON VARIABLES
TO { [ GROUP ] role_name |
PUBLIC } [, ...] [ WITH GRANT OPTION ]
3) I find the syntax in create_variable.sgml a bit too over-complicated:
CREATE [ { TEMPORARY | TEMP } ] [ { TRANSACTIONAL | TRANSACTION } ] VARIABLE [ IF NOT EXISTS ] name [ AS ] data_type ] [
COLLATE collation ]
[ NOT NULL ] [ DEFAULT default_expr ] [ { ON COMMIT DROP | ON {
TRANSACTIONAL | TRANSACTION } END RESET } ]
Do we really need both TRANSACTION and TRANSACTIONAL? Why not just one
that we already have in the grammar (i.e. TRANSACTION)?
4) I think we should rename schemavariable.h to schema_variable.h.
5) objectaddress.c has extra line after 'break;' in one switch.
6) The comment is wrong:
/*
* Find the ObjectAddress for a type or domain
*/
static ObjectAddress
get_object_address_variable(List *object, bool missing_ok)
7) I think the code/comments are really inconsistent in talking about
"variables" and "schema variables". For example in objectaddress.c we do
these two things:
case OCLASS_VARIABLE:
appendStringInfoString(, "schema variable");
break;
vs.
case DEFACLOBJ_VARIABLE:
appendStringInfoString(,
" on variables");
break;
That's going to be confusing for people.
8) I'm rather confused by CMD_PLAN_UTILITY, which seems to be defined
merely to support LET. I'm not sure why that's necessary (Why wouldn't
CMD_UTILITY be sufficient?).
Having to add conditions checking for CMD_PLAN_UTILITY to various places
in planner.c is rather annoying, and I wonder how likely it's this will
unnecessarily break external code in extensions etc.
9) This comment in planner.c seems obsolete (not updated to reflect
addition of the CMD_PLAN_UTILITY check):
/*
* If this is an INSERT/UPDATE/DELETE, and we're not being called from
* inheritance_planner, add the ModifyTable node.
*/
if (parse->commandType != CMD_SELECT && parse->commandType != CMD_PLAN_UTILITY
&& !inheritance_update)
10) I kinda wonder what happens when a function is used in a WHERE
condition, but it depends on a variable and alsu mutates it on each
call ...
11) I think Query->hasSchemaVariable (in analyze.c) should be renamed to
hasSchemaVariables (which reflects the other fields referring to things
like window functions etc.)
12) I find it rather suspicious that we make decisions in utility.c
solely based on commandType (whether it's CMD_UTILITY or not). IMO
it's pretty strange/ugly that T_LetStmt can be both CMD_UTILITY and
CMD_PLAN_UTILITY:
case T_LetStmt:
{
if (pstmt->commandType == CMD_UTILITY)
doLetStmtReset(pstmt);
else
{
Assert(pstmt->commandType == CMD_PLAN_UTILITY);
doLetStmtEval(pstmt, params, queryEnv, queryString);
}
if (completionTag)
strcpy(completionTag, "LET");
}
break;
13) Not sure why we moved DO_TABLE in
Re: proposal: schema variables
Hi po 30. 12. 2019 v 21:05 odesílatel Pavel Stehule napsal: > Hi > > po 30. 12. 2019 v 17:27 odesílatel Philippe BEAUDOIN > napsal: > >> The following review has been posted through the commitfest application: >> make installcheck-world: tested, passed >> Implements feature: tested, passed >> Spec compliant: not tested >> Documentation:tested, failed >> >> Hi Pavel, >> >> I have tested the latest version of your patch. >> Both issues I reported are now fixed. And you largely applied my >> proposals. That's great ! >> >> I have also spent some time to review more closely the documentation. I >> will send you a direct mail with an attached file for some minor comments >> on this topic. >> >> Except these documentation remarks to come, I haven't any other issue or >> suggestion to report. >> Note that I have not closely looked at the C code itself. But may be some >> other reviewers have already done that job. >> If yes, my feeling is that the patch could soon be set as "Ready for >> commiter". >> >> Best regards. Philippe. >> >> The new status of this patch is: Waiting on Author >> > > Thank you very much for your comments, and notes. Updated patch attached. > rebase > Regards > > Pavel > > schema-variables-20200117.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi po 30. 12. 2019 v 17:27 odesílatel Philippe BEAUDOIN napsal: > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, passed > Spec compliant: not tested > Documentation:tested, failed > > Hi Pavel, > > I have tested the latest version of your patch. > Both issues I reported are now fixed. And you largely applied my > proposals. That's great ! > > I have also spent some time to review more closely the documentation. I > will send you a direct mail with an attached file for some minor comments > on this topic. > > Except these documentation remarks to come, I haven't any other issue or > suggestion to report. > Note that I have not closely looked at the C code itself. But may be some > other reviewers have already done that job. > If yes, my feeling is that the patch could soon be set as "Ready for > commiter". > > Best regards. Philippe. > > The new status of this patch is: Waiting on Author > Thank you very much for your comments, and notes. Updated patch attached. Regards Pavel schema-variables-20191230.patch.gz Description: application/gzip
Re: proposal: schema variables
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation:tested, failed Hi Pavel, I have tested the latest version of your patch. Both issues I reported are now fixed. And you largely applied my proposals. That's great ! I have also spent some time to review more closely the documentation. I will send you a direct mail with an attached file for some minor comments on this topic. Except these documentation remarks to come, I haven't any other issue or suggestion to report. Note that I have not closely looked at the C code itself. But may be some other reviewers have already done that job. If yes, my feeling is that the patch could soon be set as "Ready for commiter". Best regards. Philippe. The new status of this patch is: Waiting on Author
Re: proposal: schema variables
Hi fresh rebase Regards Pavel schema-variables-20191226.patch.gz Description: application/gzip
Re: proposal: schema variables
Hi ne 22. 12. 2019 v 13:04 odesílatel Philippe BEAUDOIN napsal: > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, failed > Spec compliant: not tested > Documentation:tested, failed > > Hi Pavel, > > First of all, I would like to congratulate you for this great work. This > patch is really cool. The lack of package variables is sometimes a blocking > issue for Oracle to Postgres migrations, because the usual emulation with > GUC is sometimes not enough, in particular when there are security concerns > or when the database is used in a public cloud. > > As I look forward to having this patch commited, I decided to spend some > time to participate to the review, although I am not a C specialist and I > have not a good knowledge of the Postgres internals. Here is my report. > > A) Installation > > The patch applies correctly and the compilation is fine. The "make check" > doesn't report any issue. > > B) Basic usage > > I tried some simple schema variables use cases. No problem. > > C) The interface > > The SQL changes look good to me. > > However, in the CREATE VARIABLE command, I would replace the "TRANSACTION" > word by "TRANSACTIONAL". > > I have also tried to replace this word by a ON ROLLBACK clause at the end > of the statement, like for ON COMMIT, but I have not found a satisfying > wording to propose. > I propose compromise solution - I introduced new not reserved keyword "TRANSACTIONAL". User can use TRANSACTION or TRANSACTIONAL. It is similar relation like "TEMP" or "TEMPORAL" > > D) Behaviour > > I am ok with variables not being transactional by default. That's the most > simple, the most efficient, it emulates the package variables of other > RDBMS and it will probably fit the most common use cases. > > Note that I am not strongly opposed to having by default transactional > variables. But I don't know whether this change would be a great work. We > would have at least to find another keyword in the CREATE VARIABLE > statement. Something like "NON-TRANSACTIONAL VARIABLE" ? > > It is possible to create a NOT NULL variable without DEFAULT. When trying > to read the variable before a LET statement, one gets an error massage > saying that the NULL value is not allowed (and the documentation is clear > about this case). Just for the records, I wondered whether it wouldn't be > better to forbid a NOT NULL variable creation that wouldn't have a DEFAULT > value. But finally, I think this behaviour provides a good way to force the > variable initialisation before its use. So let's keep it as is. > > E) ACL and Rights > > I played a little bit with the GRANT and REVOKE statements. > > I have got an error (Issue 1). The following statement chain: > create variable public.sv1 int; > grant read on variable sv1 to other_user; > drop owned by other_user; > reports : ERROR: unexpected object class 4287 > should be fixed > I then tried to use DEFAULT PRIVILEGES. Despite this is not documented, I > successfuly performed: > alter default privileges in schema public grant read on variables to > simple_user; > alter default privileges in schema public grant write on variables to > simple_user; > should be fixed > When variables are then created, the grants are properly given. > And the psql \ddp command perfectly returns: > Default access privileges > Owner | Schema | Type |Access privileges > --++--+- > postgres | public | | simple_user=SW/postgres > (1 row) > > So the ALTER DEFAULT PRIVILEGES documentation chapter has to reflect this > new syntax (Issue 2). > > BTW, in the ACL, the READ privilege is represented by a S letter. A > comment in the source reports that the R letter was used in the past for > rule privilege. Looking at the postgres sources, I see that this privilege > on rules has been suppressed in 8.2, so 13 years ago. As this R letter > would be a so much better choice, I wonder whether it couldn't be reused > now for this new purpose. Is it important to keep this letter frozen ? > I use ACL_READ constant in my patch. The value of ACL_READ is defined elsewhere. So the changing from S to R should be done by separate patch and by separate discussion. > F) Extension > > I then created an extension, whose installation script creates a schema > variable and functions that use it. The schema variable is correctly linked > to the extension, so that dropping the extension drops the variable. > > But there is an issue when dumping the database (Issue 3). The script > generated by pg_dump includes the CREATE EXTENSION statement as expected > but also a redundant CREATE VARIABLE statement for the variable that > belongs to the extension. As a result, one of course gets an error at > restore time. > should be fixed now > G) Row Level Security > > I did a test activating RLS on a table and
Re: proposal: schema variables
Hi ne 22. 12. 2019 v 13:04 odesílatel Philippe BEAUDOIN napsal: > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, failed > Spec compliant: not tested > Documentation:tested, failed > > Hi Pavel, > > First of all, I would like to congratulate you for this great work. This > patch is really cool. The lack of package variables is sometimes a blocking > issue for Oracle to Postgres migrations, because the usual emulation with > GUC is sometimes not enough, in particular when there are security concerns > or when the database is used in a public cloud. > > As I look forward to having this patch commited, I decided to spend some > time to participate to the review, although I am not a C specialist and I > have not a good knowledge of the Postgres internals. Here is my report. > > A) Installation > > The patch applies correctly and the compilation is fine. The "make check" > doesn't report any issue. > > B) Basic usage > > I tried some simple schema variables use cases. No problem. > > C) The interface > > The SQL changes look good to me. > > However, in the CREATE VARIABLE command, I would replace the "TRANSACTION" > word by "TRANSACTIONAL". > There is not technical problem - the problem is in introduction new keyword "transactional" that is near to "transaction". I am not sure if it is practical to have two "similar" keyword and how much the CREATE statement has to use correct English grammar. I am not native speaker, so I am not able to see how bad is using "TRANSACTION" instead "TRANSACTIONAL" in this context. So I see a risk to have two important (it is not syntactic sugar) similar keywords. Just I afraid so using TRANSACTIONAL instead just TRANSACTION is not too user friendly. I have not strong opinion about this - and the implementation is easy, but I am not feel comfortable with introduction this keyword. > I have also tried to replace this word by a ON ROLLBACK clause at the end > of the statement, like for ON COMMIT, but I have not found a satisfying > wording to propose. > > > D) Behaviour > > I am ok with variables not being transactional by default. That's the most > simple, the most efficient, it emulates the package variables of other > RDBMS and it will probably fit the most common use cases. > > Note that I am not strongly opposed to having by default transactional > variables. But I don't know whether this change would be a great work. We > would have at least to find another keyword in the CREATE VARIABLE > statement. Something like "NON-TRANSACTIONAL VARIABLE" ? > Variables almost everywhere (global user settings - GUC is only one planet exception) are non transactional by default. I don't see any reason introduce new different design than is wide used. > It is possible to create a NOT NULL variable without DEFAULT. When trying > to read the variable before a LET statement, one gets an error massage > saying that the NULL value is not allowed (and the documentation is clear > about this case). Just for the records, I wondered whether it wouldn't be > better to forbid a NOT NULL variable creation that wouldn't have a DEFAULT > value. But finally, I think this behaviour provides a good way to force the > variable initialisation before its use. So let's keep it as is. > This is a question - and there are two possibilities postgres=# do $$ declare x int not null; begin raise notice '%', x; end; $$ ; ERROR: variable "x" must have a default value, since it's declared NOT NULL LINE 2: declare x int not null; ^ PLpgSQL requires it. But there is not a possibility to enforce future setting. So I know so behave of schema variables is little bit different, but I think so this difference has interesting use case. You can check if the variable was modified somewhere or not. > E) ACL and Rights > > I played a little bit with the GRANT and REVOKE statements. > > I have got an error (Issue 1). The following statement chain: > create variable public.sv1 int; > grant read on variable sv1 to other_user; > drop owned by other_user; > reports : ERROR: unexpected object class 4287 > this is bug and should be fixed > I then tried to use DEFAULT PRIVILEGES. Despite this is not documented, I > successfuly performed: > alter default privileges in schema public grant read on variables to > simple_user; > alter default privileges in schema public grant write on variables to > simple_user; > > When variables are then created, the grants are properly given. > And the psql \ddp command perfectly returns: > Default access privileges > Owner | Schema | Type |Access privileges > --++--+- > postgres | public | | simple_user=SW/postgres > (1 row) > > So the ALTER DEFAULT PRIVILEGES documentation chapter has to reflect this > new syntax (Issue 2). > > BTW, in the ACL, the READ
Re: proposal: schema variables
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, failed Spec compliant: not tested Documentation:tested, failed Hi Pavel, First of all, I would like to congratulate you for this great work. This patch is really cool. The lack of package variables is sometimes a blocking issue for Oracle to Postgres migrations, because the usual emulation with GUC is sometimes not enough, in particular when there are security concerns or when the database is used in a public cloud. As I look forward to having this patch commited, I decided to spend some time to participate to the review, although I am not a C specialist and I have not a good knowledge of the Postgres internals. Here is my report. A) Installation The patch applies correctly and the compilation is fine. The "make check" doesn't report any issue. B) Basic usage I tried some simple schema variables use cases. No problem. C) The interface The SQL changes look good to me. However, in the CREATE VARIABLE command, I would replace the "TRANSACTION" word by "TRANSACTIONAL". I have also tried to replace this word by a ON ROLLBACK clause at the end of the statement, like for ON COMMIT, but I have not found a satisfying wording to propose. D) Behaviour I am ok with variables not being transactional by default. That's the most simple, the most efficient, it emulates the package variables of other RDBMS and it will probably fit the most common use cases. Note that I am not strongly opposed to having by default transactional variables. But I don't know whether this change would be a great work. We would have at least to find another keyword in the CREATE VARIABLE statement. Something like "NON-TRANSACTIONAL VARIABLE" ? It is possible to create a NOT NULL variable without DEFAULT. When trying to read the variable before a LET statement, one gets an error massage saying that the NULL value is not allowed (and the documentation is clear about this case). Just for the records, I wondered whether it wouldn't be better to forbid a NOT NULL variable creation that wouldn't have a DEFAULT value. But finally, I think this behaviour provides a good way to force the variable initialisation before its use. So let's keep it as is. E) ACL and Rights I played a little bit with the GRANT and REVOKE statements. I have got an error (Issue 1). The following statement chain: create variable public.sv1 int; grant read on variable sv1 to other_user; drop owned by other_user; reports : ERROR: unexpected object class 4287 I then tried to use DEFAULT PRIVILEGES. Despite this is not documented, I successfuly performed: alter default privileges in schema public grant read on variables to simple_user; alter default privileges in schema public grant write on variables to simple_user; When variables are then created, the grants are properly given. And the psql \ddp command perfectly returns: Default access privileges Owner | Schema | Type |Access privileges --++--+- postgres | public | | simple_user=SW/postgres (1 row) So the ALTER DEFAULT PRIVILEGES documentation chapter has to reflect this new syntax (Issue 2). BTW, in the ACL, the READ privilege is represented by a S letter. A comment in the source reports that the R letter was used in the past for rule privilege. Looking at the postgres sources, I see that this privilege on rules has been suppressed in 8.2, so 13 years ago. As this R letter would be a so much better choice, I wonder whether it couldn't be reused now for this new purpose. Is it important to keep this letter frozen ? F) Extension I then created an extension, whose installation script creates a schema variable and functions that use it. The schema variable is correctly linked to the extension, so that dropping the extension drops the variable. But there is an issue when dumping the database (Issue 3). The script generated by pg_dump includes the CREATE EXTENSION statement as expected but also a redundant CREATE VARIABLE statement for the variable that belongs to the extension. As a result, one of course gets an error at restore time. G) Row Level Security I did a test activating RLS on a table and creating a POLICY that references a schema variable in its USING and WITH CHECK clauses. Everything worked fine. H) psql A \dV meta-command displays all the created variables. I would change a little bit the provided view. More precisely I would: - rename "Constraint" into "Is nullable" and report it as a boolean - rename "Special behave" into "Is transactional" and report it as a boolean - change the order of columns so to have: Schema | Name | Type | Is nullable | Default | Owner | Is transactional | Transaction end action "Is nullable" being aside "Default" I) Performance I just quickly looked at the performance, and
Re: [HACKERS] proposal: schema variables
po 18. 11. 2019 v 19:47 odesílatel Pavel Stehule napsal: > > > ne 3. 11. 2019 v 17:27 odesílatel Pavel Stehule > napsal: > >> >> >> čt 10. 10. 2019 v 11:41 odesílatel Pavel Stehule >> napsal: >> >>> Hi >>> >>> minor change - replace heap_tuple_fetch_attr by detoast_external_attr. >>> >>> >> similar update - heap_open, heap_close was replaced by table_open, >> table_close >> > > fresh rebase > only rebase Regards Pavel > Regards > > Pavel > > >> Regards >> >> Pavel >> > schema-variables-20191214.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
ne 3. 11. 2019 v 17:27 odesílatel Pavel Stehule napsal: > > > čt 10. 10. 2019 v 11:41 odesílatel Pavel Stehule > napsal: > >> Hi >> >> minor change - replace heap_tuple_fetch_attr by detoast_external_attr. >> >> > similar update - heap_open, heap_close was replaced by table_open, > table_close > fresh rebase Regards Pavel > Regards > > Pavel > schema-variables-20191118.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
čt 10. 10. 2019 v 11:41 odesílatel Pavel Stehule napsal: > Hi > > minor change - replace heap_tuple_fetch_attr by detoast_external_attr. > > similar update - heap_open, heap_close was replaced by table_open, table_close Regards Pavel schema_variables-20191103.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi minor change - replace heap_tuple_fetch_attr by detoast_external_attr. Regards Pavel pá 4. 10. 2019 v 6:12 odesílatel Pavel Stehule napsal: > Hi > > so 10. 8. 2019 v 9:10 odesílatel Pavel Stehule > napsal: > >> Hi >> >> just rebase >> > > fresh rebase > > Regards > > Pavel > > >> Regards >> >> Pavel >> > schema_variables-20191010.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi so 10. 8. 2019 v 9:10 odesílatel Pavel Stehule napsal: > Hi > > just rebase > fresh rebase Regards Pavel > Regards > > Pavel > schema-variables-20191004.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi just rebase Regards Pavel schema-variables-rebase-20190810.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi ne 30. 6. 2019 v 5:10 odesílatel Pavel Stehule napsal: > > > pá 24. 5. 2019 v 19:12 odesílatel Pavel Stehule > napsal: > >> Hi >> >> čt 9. 5. 2019 v 6:34 odesílatel Pavel Stehule >> napsal: >> >>> Hi >>> >>> rebased patch >>> >> >> rebase after pgindent >> > > fresh rebase > just rebase again Regards Pavel > Regards > > Pavel > > >> Regards >> >> Pavel >> >>> >>> Regards >>> >>> Pavel >>> >>> >>> schema-variables-20190716.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
pá 24. 5. 2019 v 19:12 odesílatel Pavel Stehule napsal: > Hi > > čt 9. 5. 2019 v 6:34 odesílatel Pavel Stehule > napsal: > >> Hi >> >> rebased patch >> > > rebase after pgindent > fresh rebase Regards Pavel > Regards > > Pavel > >> >> Regards >> >> Pavel >> >> >> schema-variables-20190630.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi čt 9. 5. 2019 v 6:34 odesílatel Pavel Stehule napsal: > Hi > > rebased patch > rebase after pgindent Regards Pavel > > Regards > > Pavel > > > schema-variables-20190524.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi rebased patch Regards Pavel schema-variables-20190509.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
fresh rebase Regards Pavel schema-variables-20180419.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
út 26. 3. 2019 v 6:40 odesílatel Pavel Stehule napsal: > Hi > > ne 24. 3. 2019 v 6:57 odesílatel Pavel Stehule > napsal: > >> Hi >> >> rebase against current master >> > > > fixed issue IF NOT EXISTS & related regress tests > another rebase Regards Pavel > Regards > > Pavel > > >> Regards >> >> Pavel >> > schema-variables-20190402.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
po 25. 3. 2019 v 20:40 odesílatel Erik Rijkers napsal: > On 2019-03-24 10:32, Pavel Stehule wrote: > > ne 24. 3. 2019 v 10:25 odesílatel Erik Rijkers napsal: > > > >> On 2019-03-24 06:57, Pavel Stehule wrote: > >> > Hi > >> > > >> > rebase against current master > >> > >> I ran into this: > >> > >> (schema 'varschema2' does not exist): > >> > >> drop variable varschema2.testv cascade; > >> ERROR: schema "varschema2" does not exist > >> create variable if not exists testv as text; > >> server closed the connection unexpectedly > >> This probably means the server terminated abnormally > >> before or while processing the request. > >> connection to server was lost > >> > >> > >> (both statements are needed to force the crash) > >> > > > > I cannot to reproduce it. > > [backtrace and stuff] > > Sorry, I don't have the wherewithal to get more info but I have repeated > this now on 4 different machines (debian jessie/stretch; centos). > > I did notice that sometimes those two offending lines > " >drop variable varschema2.testv cascade; >create variable if not exists testv as text; > " > have to be repeated a few times (never more than 4 or 5 times) before > the crash occurs (signal 11: Segmentation fault). > Should be fixed now. Thank you for report Pavel > > Erik Rijkers > > >
Re: [HACKERS] proposal: schema variables
Hi ne 24. 3. 2019 v 6:57 odesílatel Pavel Stehule napsal: > Hi > > rebase against current master > fixed issue IF NOT EXISTS & related regress tests Regards Pavel > Regards > > Pavel > schema-variables-20190326.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
On 2019-03-24 10:32, Pavel Stehule wrote: ne 24. 3. 2019 v 10:25 odesílatel Erik Rijkers napsal: On 2019-03-24 06:57, Pavel Stehule wrote: > Hi > > rebase against current master I ran into this: (schema 'varschema2' does not exist): drop variable varschema2.testv cascade; ERROR: schema "varschema2" does not exist create variable if not exists testv as text; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. connection to server was lost (both statements are needed to force the crash) I cannot to reproduce it. [backtrace and stuff] Sorry, I don't have the wherewithal to get more info but I have repeated this now on 4 different machines (debian jessie/stretch; centos). I did notice that sometimes those two offending lines " drop variable varschema2.testv cascade; create variable if not exists testv as text; " have to be repeated a few times (never more than 4 or 5 times) before the crash occurs (signal 11: Segmentation fault). Erik Rijkers
Re: [HACKERS] proposal: schema variables
ne 24. 3. 2019 v 10:25 odesílatel Erik Rijkers napsal: > On 2019-03-24 06:57, Pavel Stehule wrote: > > Hi > > > > rebase against current master > > > > I ran into this: > > (schema 'varschema2' does not exist): > > drop variable varschema2.testv cascade; > ERROR: schema "varschema2" does not exist > create variable if not exists testv as text; > server closed the connection unexpectedly > This probably means the server terminated abnormally > before or while processing the request. > connection to server was lost > > > (both statements are needed to force the crash) > I cannot to reproduce it. please, try compilation with "make distclean"; configure .. or if the problem persists, please send test case, or backtrace Regards Pavel > > > thanks, > > Erik Rijkers > > >
Re: [HACKERS] proposal: schema variables
On 2019-03-24 06:57, Pavel Stehule wrote: Hi rebase against current master I ran into this: (schema 'varschema2' does not exist): drop variable varschema2.testv cascade; ERROR: schema "varschema2" does not exist create variable if not exists testv as text; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. connection to server was lost (both statements are needed to force the crash) thanks, Erik Rijkers
Re: [HACKERS] proposal: schema variables
Hi rebase against current master Regards Pavel schema-variables-20190324.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
On 3/7/19 10:10 AM, Fabien COELHO wrote: Anyway, the patch is non trivial and very large, so targetting v12 now is indeed out of reach. Agreed. I have set the target version to PG13. Regards, -- -David da...@pgmasters.net
Re: Re: [HACKERS] proposal: schema variables
Hi >> My strong opinion based on the underlying use case is that it that such >> session variables should be transactional by default, and Pavel strong >> opinion is that they should not, to be closer to Oracle comparable >> feature. > > It is closer to any known database Oracle, DB2, Firebird, MSSQL, MySQL, Regards Pavel
Re: Re: [HACKERS] proposal: schema variables
čt 7. 3. 2019 v 9:10 odesílatel Fabien COELHO napsal: > > Hello David, > > > This patch hasn't receive any review in a while and I'm not sure if > that's > > because nobody is interested or the reviewers think it does not need any > more > > review. > > > > It seems to me that this patch as implemented does not quite satisfy any > one. > > > > I think we need to hear something from the reviewers soon or I'll push > this > > patch to PG13 as Andres recommends [1]. > > I have discussed the feature extensively with Pavel on the initial thread. > > My strong opinion based on the underlying use case is that it that such > session variables should be transactional by default, and Pavel strong > opinion is that they should not, to be closer to Oracle comparable > feature. > > According to the documentation, the current implementation does provide a > transactional feature. However, it is not the default behavior, so I'm in > disagreement on a key feature, although I do really appreciate that Pavel > implemented the transactional behavior. > > Otherwise, ISTM that they could be named "SESSION VARIABLE" because the > variable only exists in memory, in a session, and we could thing of adding > other kind of variables later on. > > I do intend to review it in depth when it is transactional by default. > I am sorry. I cannot to support this request. Variables are not transactional. My opinion is strong in this part. I would not to repeat this discussion from start. I am sorry. Regards Pavel > Anyway, the patch is non trivial and very large, so targetting v12 now is > indeed out of reach. > > -- > Fabien. > >
Re: Re: [HACKERS] proposal: schema variables
Hello David, This patch hasn't receive any review in a while and I'm not sure if that's because nobody is interested or the reviewers think it does not need any more review. It seems to me that this patch as implemented does not quite satisfy any one. I think we need to hear something from the reviewers soon or I'll push this patch to PG13 as Andres recommends [1]. I have discussed the feature extensively with Pavel on the initial thread. My strong opinion based on the underlying use case is that it that such session variables should be transactional by default, and Pavel strong opinion is that they should not, to be closer to Oracle comparable feature. According to the documentation, the current implementation does provide a transactional feature. However, it is not the default behavior, so I'm in disagreement on a key feature, although I do really appreciate that Pavel implemented the transactional behavior. Otherwise, ISTM that they could be named "SESSION VARIABLE" because the variable only exists in memory, in a session, and we could thing of adding other kind of variables later on. I do intend to review it in depth when it is transactional by default. Anyway, the patch is non trivial and very large, so targetting v12 now is indeed out of reach. -- Fabien.
Re: Re: [HACKERS] proposal: schema variables
On 3/3/19 10:27 PM, Pavel Stehule wrote: rebase and fix compilation due changes related pg_dump This patch hasn't receive any review in a while and I'm not sure if that's because nobody is interested or the reviewers think it does not need any more review. It seems to me that this patch as implemented does not quite satisfy any one. I think we need to hear something from the reviewers soon or I'll push this patch to PG13 as Andres recommends [1]. -- -David da...@pgmasters.net [1] https://www.postgresql.org/message-id/20190216054526.zss2cufdxfeudr4i%40alap3.anarazel.de
Re: [HACKERS] proposal: schema variables
Hi just rebase regards Pavel schema-variables-20190131.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi just rebase Regards Pavel schema-variables-20190130.patch.gz Description: application/gzip
Re: [HACKERS] proposal: schema variables
Hi fresh rebased patch, no other changes Pavel schema-variables-20190122-01.patch.gz Description: application/gzip
