pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: SQL-standard function body
18+ messages / 9 participants
[nested] [flat]
Commits from this thread:
2021-04-16 029c5ac03d psql: Refine lexing of BEGIN...END blocks in CREATE FUNCTION statements
2021-04-08 b3ee4c5038 Cope with NULL query string in ExecInitParallelPlan().
* pgsql: SQL-standard function body
@ 2021-04-07 19:53 Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-09 17:44 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 2 replies; 18+ messages in thread
From: Peter Eisentraut @ 2021-04-07 19:53 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
SQL-standard function body
This adds support for writing CREATE FUNCTION and CREATE PROCEDURE
statements for language SQL with a function body that conforms to the
SQL standard and is portable to other implementations.
Instead of the PostgreSQL-specific AS $$ string literal $$ syntax,
this allows writing out the SQL statements making up the body
unquoted, either as a single statement:
CREATE FUNCTION add(a integer, b integer) RETURNS integer
LANGUAGE SQL
RETURN a + b;
or as a block
CREATE PROCEDURE insert_data(a integer, b integer)
LANGUAGE SQL
BEGIN ATOMIC
INSERT INTO tbl VALUES (a);
INSERT INTO tbl VALUES (b);
END;
The function body is parsed at function definition time and stored as
expression nodes in a new pg_proc column prosqlbody. So at run time,
no further parsing is required.
However, this form does not support polymorphic arguments, because
there is no more parse analysis done at call time.
Dependencies between the function and the objects it uses are fully
tracked.
A new RETURN statement is introduced. This can only be used inside
function bodies. Internally, it is treated much like a SELECT
statement.
psql needs some new intelligence to keep track of function body
boundaries so that it doesn't send off statements when it sees
semicolons that are inside a function body.
Tested-by: Jaime Casanova <jcasanov@systemguards.com.ec>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/1c11f1eb-f00c-43b7-799d-2d44132c02d7@2ndquadrant.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/e717a9a18b2e34c9c40e5259ad4d31cd7e420750
Modified Files
--------------
doc/src/sgml/catalogs.sgml | 10 +
doc/src/sgml/ref/create_function.sgml | 125 ++++++++++--
doc/src/sgml/ref/create_procedure.sgml | 61 +++++-
src/backend/catalog/pg_aggregate.c | 1 +
src/backend/catalog/pg_proc.c | 116 +++++++----
src/backend/commands/aggregatecmds.c | 2 +
src/backend/commands/functioncmds.c | 145 ++++++++++---
src/backend/commands/typecmds.c | 4 +
src/backend/executor/functions.c | 79 +++++---
src/backend/nodes/copyfuncs.c | 15 ++
src/backend/nodes/equalfuncs.c | 13 ++
src/backend/nodes/outfuncs.c | 12 ++
src/backend/nodes/readfuncs.c | 1 +
src/backend/optimizer/util/clauses.c | 126 ++++++++----
src/backend/parser/analyze.c | 35 ++++
src/backend/parser/gram.y | 129 +++++++++---
src/backend/tcop/postgres.c | 3 +-
src/backend/utils/adt/ruleutils.c | 139 ++++++++++++-
src/bin/pg_dump/pg_dump.c | 45 ++++-
src/bin/psql/describe.c | 15 +-
src/fe_utils/psqlscan.l | 24 ++-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.dat | 4 +
src/include/catalog/pg_proc.h | 6 +-
src/include/commands/defrem.h | 2 +
src/include/executor/functions.h | 15 ++
src/include/fe_utils/psqlscan_int.h | 2 +
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h | 13 ++
src/include/parser/kwlist.h | 2 +
src/include/tcop/tcopprot.h | 1 +
src/interfaces/ecpg/preproc/ecpg.addons | 6 +
src/interfaces/ecpg/preproc/ecpg.trailer | 4 +-
src/test/regress/expected/create_function_3.out | 257 +++++++++++++++++++++++-
src/test/regress/expected/create_procedure.out | 65 ++++++
src/test/regress/sql/create_function_3.sql | 110 +++++++++-
src/test/regress/sql/create_procedure.sql | 33 +++
37 files changed, 1411 insertions(+), 212 deletions(-)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
@ 2021-04-07 22:58 ` Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 18+ messages in thread
From: Andres Freund @ 2021-04-07 22:58 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: pgsql-committers@lists.postgresql.org
Hi,
On 2021-04-07 19:53:35 +0000, Peter Eisentraut wrote:
> SQL-standard function body
>
> This adds support for writing CREATE FUNCTION and CREATE PROCEDURE
> statements for language SQL with a function body that conforms to the
> SQL standard and is portable to other implementations.
>
> Instead of the PostgreSQL-specific AS $$ string literal $$ syntax,
> this allows writing out the SQL statements making up the body
> unquoted, either as a single statement:
>
> CREATE FUNCTION add(a integer, b integer) RETURNS integer
> LANGUAGE SQL
> RETURN a + b;
>
> or as a block
>
> CREATE PROCEDURE insert_data(a integer, b integer)
> LANGUAGE SQL
> BEGIN ATOMIC
> INSERT INTO tbl VALUES (a);
> INSERT INTO tbl VALUES (b);
> END;
>
> The function body is parsed at function definition time and stored as
> expression nodes in a new pg_proc column prosqlbody. So at run time,
> no further parsing is required.
>
> However, this form does not support polymorphic arguments, because
> there is no more parse analysis done at call time.
>
> Dependencies between the function and the objects it uses are fully
> tracked.
>
> A new RETURN statement is introduced. This can only be used inside
> function bodies. Internally, it is treated much like a SELECT
> statement.
>
> psql needs some new intelligence to keep track of function body
> boundaries so that it doesn't send off statements when it sees
> semicolons that are inside a function body.
>
> Tested-by: Jaime Casanova <jcasanov@systemguards.com.ec>
> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
> Discussion: https://www.postgresql.org/message-id/flat/1c11f1eb-f00c-43b7-799d-2d44132c02d7@2ndquadrant.com
>
> Branch
> ------
> master
>
> Details
> -------
> https://git.postgresql.org/pg/commitdiff/e717a9a18b2e34c9c40e5259ad4d31cd7e420750
This is turning the BF red:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2021-04-07%2022%3A52%3A19
Might be force_parallel_mode=regress related.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
@ 2021-04-07 23:06 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` Re: pgsql: SQL-standard function body David Rowley <dgrowleyml@gmail.com>
2021-04-08 06:25 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
0 siblings, 2 replies; 18+ messages in thread
From: Tom Lane @ 2021-04-07 23:06 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
Andres Freund <andres@anarazel.de> writes:
> On 2021-04-07 19:53:35 +0000, Peter Eisentraut wrote:
>> SQL-standard function body
> This is turning the BF red:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2021-04-07%2022%3A52%3A19
> Might be force_parallel_mode=regress related.
Yeah. On my machine, it's fine without force_parallel_mode and
crashes with that. Looks like query text is not getting passed
to the parallel worker in some cases.
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-08 01:33 ` David Rowley <dgrowleyml@gmail.com>
2021-04-08 01:50 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 18+ messages in thread
From: David Rowley @ 2021-04-08 01:33 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
On Thu, 8 Apr 2021 at 11:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Andres Freund <andres@anarazel.de> writes:
>
> > Might be force_parallel_mode=regress related.
>
> Yeah. On my machine, it's fine without force_parallel_mode and
> crashes with that. Looks like query text is not getting passed
> to the parallel worker in some cases.
I wonder if it would be worth adding a regression test run of the core
tests during make check-world with force_parallel_mode set to regress.
But maybe people running the tests on slow machines might not be a fan
of that idea.
David
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` Re: pgsql: SQL-standard function body David Rowley <dgrowleyml@gmail.com>
@ 2021-04-08 01:50 ` Andres Freund <andres@anarazel.de>
2021-04-08 02:11 ` Re: pgsql: SQL-standard function body Michael Paquier <michael@paquier.xyz>
2021-04-08 14:12 ` Re: pgsql: SQL-standard function body Andrew Dunstan <andrew@dunslane.net>
0 siblings, 2 replies; 18+ messages in thread
From: Andres Freund @ 2021-04-08 01:50 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
Hi,
On 2021-04-08 13:33:18 +1200, David Rowley wrote:
> On Thu, 8 Apr 2021 at 11:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Andres Freund <andres@anarazel.de> writes:
> >
> > > Might be force_parallel_mode=regress related.
> >
> > Yeah. On my machine, it's fine without force_parallel_mode and
> > crashes with that. Looks like query text is not getting passed
> > to the parallel worker in some cases.
>
> I wonder if it would be worth adding a regression test run of the core
> tests during make check-world with force_parallel_mode set to regress.
>
> But maybe people running the tests on slow machines might not be a fan
> of that idea.
I've wondered about that too. Perhaps we could reuse the pg_upgrade run?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` Re: pgsql: SQL-standard function body David Rowley <dgrowleyml@gmail.com>
2021-04-08 01:50 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
@ 2021-04-08 02:11 ` Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 18+ messages in thread
From: Michael Paquier @ 2021-04-08 02:11 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: David Rowley <dgrowleyml@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
On Wed, Apr 07, 2021 at 06:50:17PM -0700, Andres Freund wrote:
> I've wondered about that too. Perhaps we could reuse the pg_upgrade run?
Yes, it would be a good idea to reuse that.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` Re: pgsql: SQL-standard function body David Rowley <dgrowleyml@gmail.com>
2021-04-08 01:50 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
@ 2021-04-08 14:12 ` Andrew Dunstan <andrew@dunslane.net>
2021-04-08 14:52 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 18+ messages in thread
From: Andrew Dunstan @ 2021-04-08 14:12 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
On 4/7/21 9:50 PM, Andres Freund wrote:
> Hi,
>
> On 2021-04-08 13:33:18 +1200, David Rowley wrote:
>> On Thu, 8 Apr 2021 at 11:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Andres Freund <andres@anarazel.de> writes:
>>>
>>>> Might be force_parallel_mode=regress related.
>>> Yeah. On my machine, it's fine without force_parallel_mode and
>>> crashes with that. Looks like query text is not getting passed
>>> to the parallel worker in some cases.
>> I wonder if it would be worth adding a regression test run of the core
>> tests during make check-world with force_parallel_mode set to regress.
>>
>> But maybe people running the tests on slow machines might not be a fan
>> of that idea.
> I've wondered about that too. Perhaps we could reuse the pg_upgrade run?
>
Honestly I'd prefer it if we could get rid of the rerun of 'make check'
by pg_upgrade's test.sh and instead upgrade the data directory made by
the earlier 'make check' run if it's still there (which would mean we'd
need to stop it being deleted).
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` Re: pgsql: SQL-standard function body David Rowley <dgrowleyml@gmail.com>
2021-04-08 01:50 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-08 14:12 ` Re: pgsql: SQL-standard function body Andrew Dunstan <andrew@dunslane.net>
@ 2021-04-08 14:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 14:56 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 18+ messages in thread
From: Tom Lane @ 2021-04-08 14:52 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Andres Freund <andres@anarazel.de>; David Rowley <dgrowleyml@gmail.com>; Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
Andrew Dunstan <andrew@dunslane.net> writes:
> On 4/7/21 9:50 PM, Andres Freund wrote:
>> I've wondered about that too. Perhaps we could reuse the pg_upgrade run?
> Honestly I'd prefer it if we could get rid of the rerun of 'make check'
> by pg_upgrade's test.sh and instead upgrade the data directory made by
> the earlier 'make check' run if it's still there (which would mean we'd
> need to stop it being deleted).
Good idea as far as speeding check-world and buildfarm runs, but I wonder
if we wouldn't be losing test coverage. Seeing the number of times that
buildfarm runs have gotten through "make check" only to fail at the
re-run in pg_upgrade, it seems clear to me that there is something
different about the execution environment in the latter case. I've
never been able to pin down quite what :-(
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` Re: pgsql: SQL-standard function body David Rowley <dgrowleyml@gmail.com>
2021-04-08 01:50 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-08 14:12 ` Re: pgsql: SQL-standard function body Andrew Dunstan <andrew@dunslane.net>
2021-04-08 14:52 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-08 14:56 ` Andres Freund <andres@anarazel.de>
2021-04-09 00:27 ` Re: pgsql: SQL-standard function body Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 18+ messages in thread
From: Andres Freund @ 2021-04-08 14:56 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Hi,
On Thu, Apr 8, 2021, at 07:52, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > On 4/7/21 9:50 PM, Andres Freund wrote:
> >> I've wondered about that too. Perhaps we could reuse the pg_upgrade run?
>
> > Honestly I'd prefer it if we could get rid of the rerun of 'make check'
> > by pg_upgrade's test.sh and instead upgrade the data directory made by
> > the earlier 'make check' run if it's still there (which would mean we'd
> > need to stop it being deleted).
>
> Good idea as far as speeding check-world and buildfarm runs, but I wonder
> if we wouldn't be losing test coverage. Seeing the number of times that
> buildfarm runs have gotten through "make check" only to fail at the
> re-run in pg_upgrade, it seems clear to me that there is something
> different about the execution environment in the latter case. I've
> never been able to pin down quite what :-(
IIRC we made it run with fsync explicitly enabled. Which obviously will change timing somewhat substantially...
Andres
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` Re: pgsql: SQL-standard function body David Rowley <dgrowleyml@gmail.com>
2021-04-08 01:50 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-08 14:12 ` Re: pgsql: SQL-standard function body Andrew Dunstan <andrew@dunslane.net>
2021-04-08 14:52 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 14:56 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
@ 2021-04-09 00:27 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 18+ messages in thread
From: Michael Paquier @ 2021-04-09 00:27 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-committers@lists.postgresql.org
On Thu, Apr 08, 2021 at 07:56:10AM -0700, Andres Freund wrote:
> IIRC we made it run with fsync explicitly enabled. Which obviously
> will change timing somewhat substantially...
Yeah. I think that this kind of things pretty much boils down into
making the tests of pg_upgrade into TAP. We have already a set of
rather-sane defaults in PostgresNode. This does not prevent just
sticking one or two extra configuration lines into test.sh, of
course.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-08 06:25 ` Peter Eisentraut <peter@eisentraut.org>
2021-04-08 12:30 ` Re: pgsql: SQL-standard function body Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 18+ messages in thread
From: Peter Eisentraut @ 2021-04-08 06:25 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; +Cc: pgsql-committers@lists.postgresql.org
On 08.04.21 01:06, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2021-04-07 19:53:35 +0000, Peter Eisentraut wrote:
>>> SQL-standard function body
>
>> This is turning the BF red:
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2021-04-07%2022%3A52%3A19
>> Might be force_parallel_mode=regress related.
>
> Yeah. On my machine, it's fine without force_parallel_mode and
> crashes with that. Looks like query text is not getting passed
> to the parallel worker in some cases.
There does not appear to be any indication in CreateQueryDesc() or for
es_sourceText that the query source text needs to be not NULL (unlike
PortalDefineQuery() for example). If we want that to be the case, an
assert in CreateQueryDesc() would be a straightforward way to catch
this. But for example code in CreateExecutorState() and
executor_errposition() is prepared to have es_sourceText be NULL, so
relative to that, the fix that Andres put into ExecInitParallelPlan() to
handle that as well seems appropriate.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Re: pgsql: SQL-standard function body Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Re: pgsql: SQL-standard function body Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 06:25 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
@ 2021-04-08 12:30 ` Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 18+ messages in thread
From: Robert Haas @ 2021-04-08 12:30 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; pgsql-committers <pgsql-committers@lists.postgresql.org>
On Thu, Apr 8, 2021 at 2:25 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> On 08.04.21 01:06, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> >> On 2021-04-07 19:53:35 +0000, Peter Eisentraut wrote:
> >>> SQL-standard function body
> >
> >> This is turning the BF red:
> >> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2021-04-07%2022%3A52%3A19
> >> Might be force_parallel_mode=regress related.
> >
> > Yeah. On my machine, it's fine without force_parallel_mode and
> > crashes with that. Looks like query text is not getting passed
> > to the parallel worker in some cases.
>
> There does not appear to be any indication in CreateQueryDesc() or for
> es_sourceText that the query source text needs to be not NULL (unlike
> PortalDefineQuery() for example). If we want that to be the case, an
> assert in CreateQueryDesc() would be a straightforward way to catch
> this. But for example code in CreateExecutorState() and
> executor_errposition() is prepared to have es_sourceText be NULL, so
> relative to that, the fix that Andres put into ExecInitParallelPlan() to
> handle that as well seems appropriate.
I think we should insist on having a query string all the time, if at
all possible, because having the query string is awfully nice for
debugging. For example, if a parallel worker dumps core, you'd like to
be able to find out what query it was trying to run when things went
bad. But if we're not going to do that, then I think it would be
better to do something more like what Noah did in
f90e80b9138355a51d2d5b5b63e1f89c4ba53325 rather than what Andres did
here.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
@ 2021-04-09 17:44 ` Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-10 01:38 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
1 sibling, 1 reply; 18+ messages in thread
From: Laurenz Albe @ 2021-04-09 17:44 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
On Wed, 2021-04-07 at 19:53 +0000, Peter Eisentraut wrote:
> SQL-standard function body
>
> This adds support for writing CREATE FUNCTION and CREATE PROCEDURE
> statements for language SQL with a function body that conforms to the
> SQL standard and is portable to other implementations.
>
> [...]
>
> psql needs some new intelligence to keep track of function body
> boundaries so that it doesn't send off statements when it sees
> semicolons that are inside a function body.
This causes psql to fail to recognize the semicolon in the following
statement as the end of a statement:
IMPORT FOREIGN SCHEMA x FROM SERVER y INTO z OPTIONS (case 'lower');
The cause is the "case", which is recognized as the start of a function
body.
Now you could argue that "case" is a reserved word, and I should
double quote it if I insist on using it as a FDW option, but it
is a regression.
Would it be an option to recognize BEGIN and CASE as starting a
function body only if they are *not* inside parentheses, like in
the attached?
Yours,
Laurenz Albe
Attachments:
0001-Improve-psql-s-parsing-of-SQL-standard-function-bodi.patch (text/x-patch, 1.1K) download
From 8c95440905c01b8c9ed8b46ef01a24b01f24580d Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Fri, 9 Apr 2021 19:40:36 +0200
Subject: [PATCH] Improve psql's parsing of SQL standard function bodies
Treat BEGIN and CASE as the start of a function body only
if we are not inside parentheses.
That allows statements like the following to be parsed correctly:
ALTER FOREIGN TABLE x OPTIONS (ADD case 'upper');
---
src/fe_utils/psqlscan.l | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index 4ec57e96a9..dbfcfc84e5 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -871,8 +871,9 @@ other .
{identifier} {
cur_state->identifier_count++;
- if (pg_strcasecmp(yytext, "begin") == 0
- || pg_strcasecmp(yytext, "case") == 0)
+ if (cur_state->paren_depth == 0
+ && (pg_strcasecmp(yytext, "begin") == 0
+ || pg_strcasecmp(yytext, "case") == 0))
{
if (cur_state->identifier_count > 1)
cur_state->begin_depth++;
--
2.26.3
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-09 17:44 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
@ 2021-04-10 01:38 ` Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-12 15:25 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 1 reply; 18+ messages in thread
From: Laurenz Albe @ 2021-04-10 01:38 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; pgsql-committers@lists.postgresql.org
On Fri, 2021-04-09 at 19:44 +0200, I wrote:
> > SQL-standard function body
> >
> > psql needs some new intelligence to keep track of function body
> > boundaries so that it doesn't send off statements when it sees
> > semicolons that are inside a function body.
>
> This causes psql to fail to recognize the semicolon in the following
> statement as the end of a statement:
>
> IMPORT FOREIGN SCHEMA x FROM SERVER y INTO z OPTIONS (case 'lower');
>
> The cause is the "case", which is recognized as the start of a function
> body.
>
> Would it be an option to recognize BEGIN and CASE as starting a
> function body only if they are *not* inside parentheses, like in
> the attached?
Here is an improved patch, which treats END in the same fashion
(not properly indented for readability).
Yours,
Laurenz Albe
Attachments:
0001-Improve-parsing-of-SQL-standard-functions-in-psql.patch (text/x-patch, 1.0K) download
From b0aaf040a42e3a036ba43c9891e07bad73c35a5e Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Sat, 10 Apr 2021 03:35:39 +0200
Subject: [PATCH] Improve parsing of SQL standard functions in psql
Handle BEGIN, CASE and END as special only if we are not inside parentheses.
That allows statements like the following to be parsed correctly:
ALTER FOREIGN TABLE x OPTIONS (ADD case 'upper');
---
src/fe_utils/psqlscan.l | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index 4ec57e96a9..caf6412164 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -871,6 +871,8 @@ other .
{identifier} {
cur_state->identifier_count++;
+ if (cur_state->paren_depth == 0)
+ {
if (pg_strcasecmp(yytext, "begin") == 0
|| pg_strcasecmp(yytext, "case") == 0)
{
@@ -882,6 +884,7 @@ other .
if (cur_state->begin_depth > 0)
cur_state->begin_depth--;
}
+ }
ECHO;
}
--
2.26.3
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-09 17:44 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-10 01:38 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
@ 2021-04-12 15:25 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-04-12 19:00 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 18+ messages in thread
From: Peter Eisentraut @ 2021-04-12 15:25 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; pgsql-committers@lists.postgresql.org
On 10.04.21 03:38, Laurenz Albe wrote:
> On Fri, 2021-04-09 at 19:44 +0200, I wrote:
>>> SQL-standard function body
>>>
>>> psql needs some new intelligence to keep track of function body
>>> boundaries so that it doesn't send off statements when it sees
>>> semicolons that are inside a function body.
>>
>> This causes psql to fail to recognize the semicolon in the following
>> statement as the end of a statement:
>>
>> IMPORT FOREIGN SCHEMA x FROM SERVER y INTO z OPTIONS (case 'lower');
>>
>> The cause is the "case", which is recognized as the start of a function
>> body.
>>
>> Would it be an option to recognize BEGIN and CASE as starting a
>> function body only if they are *not* inside parentheses, like in
>> the attached?
>
> Here is an improved patch, which treats END in the same fashion
> (not properly indented for readability).
Thanks, I took another look at this and augmented your change with a
change that tracks whether the statement starts with CREATE [OR REPLACE]
{FUNCTION|PROCEDURE}. That should make it pretty safe. What do you think?
Attachments:
0001-psql-Refine-lexing-of-BEGIN.END-blocks-in-CREATE-FUN.patch (text/plain, 3.9K) download
From 26251be9be89583278573ed28540e5f16f2616fb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 12 Apr 2021 17:19:59 +0200
Subject: [PATCH] psql: Refine lexing of BEGIN...END blocks in CREATE FUNCTION
statements
Only track BEGIN...END blocks if they are in a CREATE [OR REPLACE]
{FUNCTION|PROCEDURE} statement. Ignore if in parentheses.
Reported-by: Laurenz Albe <laurenz.albe@cybertec.at>
---
src/fe_utils/psqlscan.l | 53 ++++++++++++++++++++++++-----
src/include/fe_utils/psqlscan_int.h | 8 ++++-
2 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index 4ec57e96a9..991b7de0b5 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -870,18 +870,55 @@ other .
{identifier} {
- cur_state->identifier_count++;
- if (pg_strcasecmp(yytext, "begin") == 0
- || pg_strcasecmp(yytext, "case") == 0)
+ /*
+ * We need to track if we are inside a BEGIN .. END block
+ * in a function definition, so that semicolons contained
+ * therein don't terminate the whole statement. Short of
+ * writing a full parser here, the following heuristic
+ * should work. First, we track whether the beginning of
+ * the statement matches CREATE [OR REPLACE]
+ * {FUNCTION|PROCEDURE}
+ */
+
+ if (cur_state->identifier_count == 0)
+ memset(cur_state->identifiers, 0, sizeof(cur_state->identifiers));
+
+ if (pg_strcasecmp(yytext, "create") == 0 ||
+ pg_strcasecmp(yytext, "function") == 0 ||
+ pg_strcasecmp(yytext, "procedure") == 0 ||
+ pg_strcasecmp(yytext, "or") == 0 ||
+ pg_strcasecmp(yytext, "replace") == 0)
{
- if (cur_state->identifier_count > 1)
- cur_state->begin_depth++;
+ if (cur_state->identifier_count < sizeof(cur_state->identifiers))
+ cur_state->identifiers[cur_state->identifier_count] = pg_tolower((unsigned char) yytext[0]);
}
- else if (pg_strcasecmp(yytext, "end") == 0)
+
+ cur_state->identifier_count++;
+
+ if (cur_state->identifiers[0] == 'c' &&
+ (cur_state->identifiers[1] == 'f' || cur_state->identifiers[1] == 'p' ||
+ (cur_state->identifiers[1] == 'o' && cur_state->identifiers[2] == 'r' &&
+ (cur_state->identifiers[3] == 'f' || cur_state->identifiers[3] == 'p'))) &&
+ cur_state->paren_depth == 0)
{
- if (cur_state->begin_depth > 0)
- cur_state->begin_depth--;
+ if (pg_strcasecmp(yytext, "begin") == 0)
+ cur_state->begin_depth++;
+ else if (pg_strcasecmp(yytext, "case") == 0)
+ {
+ /*
+ * CASE also ends with END. We only need to track
+ * this if we are already inside a BEGIN.
+ */
+ if (cur_state->begin_depth >= 1)
+ cur_state->begin_depth++;
+ }
+ else if (pg_strcasecmp(yytext, "end") == 0)
+ {
+ if (cur_state->begin_depth > 0)
+ cur_state->begin_depth--;
+ }
}
+
ECHO;
}
diff --git a/src/include/fe_utils/psqlscan_int.h b/src/include/fe_utils/psqlscan_int.h
index 91d7d4d5c6..434e554961 100644
--- a/src/include/fe_utils/psqlscan_int.h
+++ b/src/include/fe_utils/psqlscan_int.h
@@ -114,8 +114,14 @@ typedef struct PsqlScanStateData
int paren_depth; /* depth of nesting in parentheses */
int xcdepth; /* depth of nesting in slash-star comments */
char *dolqstart; /* current $foo$ quote start string */
+
+ /*
+ * State to track boundaries of BEGIN ... END blocks in function
+ * definitions, so that semicolons do not send query too early.
+ */
int identifier_count; /* identifiers since start of statement */
- int begin_depth; /* depth of begin/end routine body blocks */
+ char identifiers[4]; /* records the first few identifiers */
+ int begin_depth; /* depth of begin/end pairs */
/*
* Callback functions provided by the program making use of the lexer,
--
2.31.1
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-09 17:44 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-10 01:38 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-12 15:25 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter.eisentraut@enterprisedb.com>
@ 2021-04-12 19:00 ` Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-16 10:26 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 1 reply; 18+ messages in thread
From: Laurenz Albe @ 2021-04-12 19:00 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-committers@lists.postgresql.org
On Mon, 2021-04-12 at 17:25 +0200, Peter Eisentraut wrote:
> On 10.04.21 03:38, Laurenz Albe wrote:
> > On Fri, 2021-04-09 at 19:44 +0200, I wrote:
> > > > SQL-standard function body
> > > >
> > > > psql needs some new intelligence to keep track of function body
> > > > boundaries so that it doesn't send off statements when it sees
> > > > semicolons that are inside a function body.
> > >
> > > This causes psql to fail to recognize the semicolon in the following
> > > statement as the end of a statement:
> > >
> > > IMPORT FOREIGN SCHEMA x FROM SERVER y INTO z OPTIONS (case 'lower');
> > >
> > > The cause is the "case", which is recognized as the start of a function
> > > body.
> > >
> > > Would it be an option to recognize BEGIN and CASE as starting a
> > > function body only if they are *not* inside parentheses, like in
> > > the attached?
> >
> > Here is an improved patch, which treats END in the same fashion
> > (not properly indented for readability).
>
> Thanks, I took another look at this and augmented your change with a
> change that tracks whether the statement starts with CREATE [OR REPLACE]
> {FUNCTION|PROCEDURE}. That should make it pretty safe. What do you think?
Thanks, that is fine with me and more than I asked for.
That should definitely exclude all false positive matches.
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-09 17:44 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-10 01:38 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-12 15:25 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-04-12 19:00 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
@ 2021-04-16 10:26 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-04-16 14:09 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 18+ messages in thread
From: Peter Eisentraut @ 2021-04-16 10:26 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: pgsql-committers@lists.postgresql.org
On 12.04.21 21:00, Laurenz Albe wrote:
>>> Here is an improved patch, which treats END in the same fashion
>>> (not properly indented for readability).
>>
>> Thanks, I took another look at this and augmented your change with a
>> change that tracks whether the statement starts with CREATE [OR REPLACE]
>> {FUNCTION|PROCEDURE}. That should make it pretty safe. What do you think?
>
> Thanks, that is fine with me and more than I asked for.
> That should definitely exclude all false positive matches.
committed
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: pgsql: SQL-standard function body
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-09 17:44 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-10 01:38 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-12 15:25 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-04-12 19:00 ` Re: pgsql: SQL-standard function body Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-16 10:26 ` Re: pgsql: SQL-standard function body Peter Eisentraut <peter.eisentraut@enterprisedb.com>
@ 2021-04-16 14:09 ` Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 0 replies; 18+ messages in thread
From: Laurenz Albe @ 2021-04-16 14:09 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; +Cc: pgsql-committers@lists.postgresql.org
On Fri, 2021-04-16 at 12:26 +0200, Peter Eisentraut wrote:
> committed
Thank you!
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2021-04-16 14:09 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 19:53 pgsql: SQL-standard function body Peter Eisentraut <peter@eisentraut.org>
2021-04-07 22:58 ` Andres Freund <andres@anarazel.de>
2021-04-07 23:06 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 01:33 ` David Rowley <dgrowleyml@gmail.com>
2021-04-08 01:50 ` Andres Freund <andres@anarazel.de>
2021-04-08 02:11 ` Michael Paquier <michael@paquier.xyz>
2021-04-08 14:12 ` Andrew Dunstan <andrew@dunslane.net>
2021-04-08 14:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-08 14:56 ` Andres Freund <andres@anarazel.de>
2021-04-09 00:27 ` Michael Paquier <michael@paquier.xyz>
2021-04-08 06:25 ` Peter Eisentraut <peter@eisentraut.org>
2021-04-08 12:30 ` Robert Haas <robertmhaas@gmail.com>
2021-04-09 17:44 ` Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-10 01:38 ` Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-12 15:25 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-04-12 19:00 ` Laurenz Albe <laurenz.albe@cybertec.at>
2021-04-16 10:26 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2021-04-16 14:09 ` Laurenz Albe <laurenz.albe@cybertec.at>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox