psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

Started by David Adamsabout 3 years ago9 messagesbugs
Jump to latest
#1David Adams
dpadams@gmail.com

BEGIN ATOMIC is one of my favorite additions ot Postgres 14, and I use it
widely. Mostly with CREATE FUNCTION, and sometimes with CREATE PROCEDURE.
It's reassuring to have table and column references baked in as referenced,
rather than stored as text. I can't count how many times the dependency
tracking on views has stopped me from putting things into an inconsistent
state.

I've found a fair few 3rd party tools have, or have had, trouble parsing
and processing functions and procedures that use BEGIN ATOMIC. And,
unfortunately, this seems to be the case with psql too.

My working routines are fairly long, so I've built out a toy example that I
think illustrates the problem. Below are two function definitions, entered
and run from a GUI console:

DROP FUNCTION IF EXISTS tell_me_how_plain();
CREATE OR REPLACE FUNCTION tell_me_how_plain()

RETURNS text

LANGUAGE SQL

return 'plain';

END;

DROP FUNCTION IF EXISTS tools.tell_me_how_atomic();
CREATE OR REPLACE FUNCTION tools.tell_me_how_atomic()

RETURNS text

LANGUAGE SQL

BEGIN ATOMIC

return 'atomic';

END;

select * from tell_me_how_plain(); -- plain
select * from tell_me_how_atomic(); -- atomic

If I run the script files from the command line with psql, the
tell_me_how_plain.sql function builds fine, and the
tell_me_how_atomic.sql throws a bogus syntax error:

ERROR: syntax error at end of input
LINE 5: return 'atomic';
^

I've tried using both the -f flag and the path, and the < path option, like so:

psql -p 5555 -U postgres -d squid -w -v --echo-all -f
"/Users/dpadams2/Desktop/PG_Bug_Tell_Me_How/tell_me_how_atomic.sql"

psql -p 5555 -U postgres -d squid -w -v --echo-all <
"/Users/dpadams2/Desktop/PG_Bug_Tell_Me_How/tell_me_how_atomic.sql"

I really hope that this is *not a bug*, and that I'm missing something
obvious that gets around this error. I noticed in the release notes
for 14.7 and 15.2

that there were changes to psql related to BEGIN ATOMIC on a few
flags, but not -f.

Any help appreciated, or a confirmation so that I can stop trying to
fix this on my own.

Thanks.

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Adams (#1)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

David Adams <dpadams@gmail.com> writes:

CREATE OR REPLACE FUNCTION tools.tell_me_how_atomic()
RETURNS text
LANGUAGE SQL
BEGIN ATOMIC
return 'atomic';
END;
[ throws a syntax error ]

This works for me. I wonder if you have some issues with bogus
invisible characters in your actual input file. Copying and
pasting from your mail (but removing the schema name for
convenience), I got

regression=# CREATE OR REPLACE FUNCTION tell_me_how_atomic()
regression-#
regression-# RETURNS text
regression-#
regression-# LANGUAGE SQL
regression-#
regression-# BEGIN ATOMIC
regression-#
regression-# return 'atomic';
regression-#
regression-# END;
CREATE FUNCTION
regression=# select * from tell_me_how_atomic()
regression-# ;
tell_me_how_atomic
--------------------
atomic
(1 row)

regards, tom lane

#3David G. Johnston
david.g.johnston@gmail.com
In reply to: David Adams (#1)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

On Sunday, March 12, 2023, David Adams <dpadams@gmail.com> wrote:

DROP FUNCTION IF EXISTS tools.tell_me_how_atomic();
CREATE OR REPLACE FUNCTION tools.tell_me_how_atomic()

RETURNS text

LANGUAGE SQL

BEGIN ATOMIC

return 'atomic';

END;

Not a bug, user-error.

https://www.postgresql.org/docs/current/sql-createfunction.html

There are two valid function body forms defined under “sql_body”. Yours is
neither of those. Rewrite your body using one of those forms. Removing
being/end would make sense for this example.

David J.

#4David Adams
dpadams@gmail.com
In reply to: Tom Lane (#2)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

Thanks for the instantaneous reply.

As far as I know, the problem only shows up with reading in a file with -f
or <: That's a bit different to entering the lines one by one.

psql -p 5555 -U postgres -d squid -w -v --echo-all -f
"/Users/dpadams2/Desktop/PG_Bug_Tell_Me_How/tell_me_how_atomic.sql"

I've checked again, and can't find any gremlins/invisible characters in the
source file, attached. And, this only happens with scripts that use BEGIN
ATOMIC, and it seems to happen on all of those files.

For background, I've got my source in a directory tree, and some custom
scripts and magic hinting files to let me write out a series of psql -f
commands to rebuild my database from scratch. It's handy for linting, and
various setup and test procedures.

On Mon, Mar 13, 2023 at 3:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Show quoted text

David Adams <dpadams@gmail.com> writes:

CREATE OR REPLACE FUNCTION tools.tell_me_how_atomic()
RETURNS text
LANGUAGE SQL
BEGIN ATOMIC
return 'atomic';
END;
[ throws a syntax error ]

This works for me. I wonder if you have some issues with bogus
invisible characters in your actual input file. Copying and
pasting from your mail (but removing the schema name for
convenience), I got

regression=# CREATE OR REPLACE FUNCTION tell_me_how_atomic()
regression-#
regression-# RETURNS text
regression-#
regression-# LANGUAGE SQL
regression-#
regression-# BEGIN ATOMIC
regression-#
regression-# return 'atomic';
regression-#
regression-# END;
CREATE FUNCTION
regression=# select * from tell_me_how_atomic()
regression-# ;
tell_me_how_atomic
--------------------
atomic
(1 row)

regards, tom lane

Attachments:

tell_me_how_atomic.sqlapplication/octet-stream; name=tell_me_how_atomic.sqlDownload
#5David Adams
dpadams@gmail.com
In reply to: David G. Johnston (#3)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

Thanks for the answer, a user error would be best case for me as then I can
fix it.

I was hoping that cutting down my real routines to a simple example
wouldn't introduce errors, maybe it did. The form I'm following is shown
the docs like so:

```
BEGIN ATOMIC
statement;
statement;
...
statement;
END
```

My code is

```
BEGIN ATOMIC

return 'atomic';

END;
```

Apart from the final semi-colon, it's exactly the same form, as far as I
can see. Taking the semi-colon out doesn't change anything.

In this toy example, `BEGIN ATOMIC` serves no purpose. In my real code,
`BEGIN ATOMIC` is *very* helpful, as it puts table and column refernces
into the autoamtic dependency tree.

Not sure what I'm missing here, hope it really is something simple.

On Mon, Mar 13, 2023 at 3:47 PM David G. Johnston <
david.g.johnston@gmail.com> wrote:

Show quoted text

On Sunday, March 12, 2023, David Adams <dpadams@gmail.com> wrote:

DROP FUNCTION IF EXISTS tools.tell_me_how_atomic();
CREATE OR REPLACE FUNCTION tools.tell_me_how_atomic()

RETURNS text

LANGUAGE SQL

BEGIN ATOMIC

return 'atomic';

END;

Not a bug, user-error.

https://www.postgresql.org/docs/current/sql-createfunction.html

There are two valid function body forms defined under “sql_body”. Yours
is neither of those. Rewrite your body using one of those forms. Removing
being/end would make sense for this example.

David J.

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: David Adams (#4)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

David Adams <dpadams@gmail.com> writes:

As far as I know, the problem only shows up with reading in a file with -f
or <: That's a bit different to entering the lines one by one.

No joy on that either ...

$ cat test.sql
CREATE OR REPLACE FUNCTION tell_me_how_atomic()
RETURNS text
LANGUAGE SQL
BEGIN ATOMIC
return 'atomic';
END;
$ psql -f test.sql
CREATE FUNCTION
$ cat test.sql | psql
CREATE FUNCTION

I've checked again, and can't find any gremlins/invisible characters in the
source file, attached. And, this only happens with scripts that use BEGIN
ATOMIC, and it seems to happen on all of those files.

Are you entirely sure you're using a new-enough psql? I can replicate
your symptom if I try to use v13 psql to send this command to a
v14-or-later server.

regards, tom lane

#7David G. Johnston
david.g.johnston@gmail.com
In reply to: David Adams (#5)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

On Sun, Mar 12, 2023 at 9:57 PM David Adams <dpadams@gmail.com> wrote:

Thanks for the answer, a user error would be best case for me as then I
can fix it.

Tom is probably on the right track here with psql versions.

I still say this shouldn't work per the documentation since "return" isn't
a valid SQL statement, if you want to use "begin atomic" write "SELECT
'result';" instead as the final statement of the function. The "return"
syntax is shown to only work with the "LANGUAGE SQL RETURN expression;"
format. Though since this does in fact work the docs probably should be
tweaked instead.

David J.

#8David Adams
dpadams@gmail.com
In reply to: Tom Lane (#6)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

Are you entirely sure you're using a new-enough psql?

Darn kids, got into my command line and messed up my path ;-)

THANK YOU!

Problem was in front of keyboard, for sure.

#9David Adams
dpadams@gmail.com
In reply to: David G. Johnston (#7)
Re: psql 14.7/15.2 report a bogus syntax error on function and procedure files that use BEGIN ATOMIC

Tom is probably on the right track here with psql versions.

Yes!

I still say this shouldn't work per the documentation since "return"

isn't a valid SQL statement,
Right you are. Just looked at my most common use, and it's returning a
SELECT. I write short functions and procedures by hand, but mostly I write
code generators to punch them out for me. In my most common case for BEGIN
ATOMIC, I've got a setup like this:

* Create a view, which creates a compound type by proxy.
* Create a function that takes an array of my_foo_v1[].
* From the client side, package data formatted according to the rules of
the my_foo_v1 format.
* Submit those arrays to insert_my_foo_v1(my_foo_v1[])

That function unnests the data into an in-memory row set, then INSERT...ON
CONFLICTs everything into the target table. I'm trying to convert to PG 15
now, as we have some cases where we'll need MERGE instead. (Specifically,
when we want to maintain a globally unique ID across partitions divided by
date.)

Each field appears 2-3 times in the generated function in the extraction,
insert, and on conflict clauses.....3 times for on conflict, I think that
it will only be twice for MERGE, haven't written that yet past the toy
stage.

So, you can see why I appreciate BEGIN ATOMIC. And, for the ON CONFLICT
version, the return is a small object including the submitted row count,
inserted row count, and estimated update count. MERGE doesn't support
RETURNING, so I'll have to live without it.

RETURNING is such a gem, it ought to be added to the standards.

If you're wondering "why go to all that work?", it's so that we can support
multiple format versions on inserts on a table simultaneously. Because
lagging clients. I machine-generate it, so it doesn't take long at all,
once the generator is written. Plus, I'm a fanatic for type checking.
Postgres' type features are absolutely fantastic, spoiling me forever for
other databases.. I assume that other people have ORMs to do this sort of
magic for them, but I didn't go that way.

On Mon, Mar 13, 2023 at 4:04 PM David G. Johnston <
david.g.johnston@gmail.com> wrote:

Show quoted text

On Sun, Mar 12, 2023 at 9:57 PM David Adams <dpadams@gmail.com> wrote:

Thanks for the answer, a user error would be best case for me as then I
can fix it.

Tom is probably on the right track here with psql versions.

I still say this shouldn't work per the documentation since "return" isn't
a valid SQL statement, if you want to use "begin atomic" write "SELECT
'result';" instead as the final statement of the function. The "return"
syntax is shown to only work with the "LANGUAGE SQL RETURN expression;"
format. Though since this does in fact work the docs probably should be
tweaked instead.

David J.