Re: [PATCH] Provide support for trailing commas

On 25.08.26 03:38, Greg Sabino Mullane wrote:
> Sorry for the delay. Please find attached a patch to only support
> target_list. I added a small test, but could not really find an
> appropriate spot in the docs to mention this. Suggestions welcome.

I have looked at this patch. It works mechanically, but it needs some
refinement.

Note that the target_list grammar production is not only used in the
SELECT statement but also in the RETURNING clause, and in PL/pgSQL.
Especially the latter needs to be properly explored, and tests should be
added for all these cases, if we decide to keep them, otherwise the
grammar changes need to be refined.

The tests could also be expanded to cover some error cases like

select 1,, from ...

For the documentation, at least in

doc/src/sgml/ref/select.sgml

and in

doc/src/sgml/syntax.sgml

in the section "Special Characters". There is a specific mention there
of commas to separate list elements. This could be a good place to
discuss that trailing commas are allowed in some cases and not others.

The gram.y changes have a number of whitespace inconsistencies. This
file is not covered by pgindent, so please manually make sure that
whitespace is consistent with existing code.

You don't need a %type declaration for opt_trailing_comma if you are not
going to assign anything interesting to it. Just leaving it off is fine.

Putting the opt_trailing_comma production under "Generic supporting
productions for DDL" seems incorrect.

Also, the production opt_trailing_comma is actually just "opt_comma" if
you think about it. It doesn't enforce that it's trailing.

And alternative way to write the grammar could be without an extra
production for the comma, like

target_list:
target_list_items ',' { $$ = $1; }
| target_list_items { $$ = $1; }
;

Maybe that's simpler?