[PATCH] Provide support for trailing commas - Mailing list pgsql-hackers

From Greg Sabino Mullane
Subject [PATCH] Provide support for trailing commas
Date
Msg-id CAKAnmmKVwP=yr8t8p8_QtrBFjyENLZyPxscJh4UrkCeqMQuhrw@mail.gmail.com
Whole thread Raw
Responses Re: [PATCH] Provide support for trailing commas
Re: [PATCH] Provide support for trailing commas
List pgsql-hackers
tl;dr Provide support for trailing commas, where possible and practical.

(Disclaimer: No LLM or AI was used to craft this patch or this email)

Happy New Year! Please find attached a patch to provide comprehensive trailing comma support, where we allow (when possible) a comma that does NOT come before the next item in a list. To wit:

SELECT 1,2,3, FROM pg_class, ORDER BY greatest(relpages,1,);

This is not as trivial a task as it seems, as those who have dabbled with our parser (or parsers in general) may suspect. At the end of the day, however, 99% of the places in which we use a comma for a list of items now have support for trailing commas.

Most of this email is an item by item breakdown of each occurance of commas with the gram.y file (in order), how it was solved, and a quick example showing the newly supported syntax.

The overall approach is to break the existing lists into two lists, the new one with a suffix of "_items", and put the optional trailing comma at the end of that new list. For example, here's how I solved utility_option_list. It used to look like this:

utility_option_list:
  utility_option_elem  { $$ = list_make1($1); }
  | utility_option_list ',' utility_option_elem  { $$ = lappend($1, $3); }
;

Now it looks like this:

utility_option_list:
  utility_option_list_items opt_trailing_comma  { $$ = $1; }
;

utility_option_list_items:
  utility_option_elem  { $$ = list_make1($1); }
  | utility_option_list_items ',' utility_option_elem  { $$ = lappend($1, $3); }
;

Where opt_trailing_comma is:

opt_trailing_comma:
  ','            { $$ = NULL; }
  | /* EMPTY */  { $$ = NULL; }
;

That system satisfies most of the grammar, but there were some problematic spots that caused the ol' shift/reduce conflict that is the bane of those editing gram.y.

First, I made our commas a little more special with:

%left  ','

Second, I added some new items to the %nonassoc lists. They are detailed below as added, but the basic need is because lists of things in Postgres often have some optional modifiers. For example, TRUNCATE takes a list of table to truncate, but also allows the CASCADE keyword. Further, we do not prevent people from creating a table named "cascade", so this command will truncate two tables:

TRUNCATE TABLE foobar, cascade;

This command will truncate one table, and cascade to other tables as needed:

TRUNCATE TBALE foobar cascade;

Trailing comma allows us to call a list of tables, and end with a comma:

TRUNCATE TBALE foobar, foobaz, ;

However, we now have some ambiguity (i.e. shift/reduce conflicts) because "TRUNCATE TABLE foobar, cascade;" can now indicate truncating two tables, or one table (with a trailing comma), plus the cascade keyword. This is solved by this patch in favor of the latter interpretation. In other words, when there is a trailing comma, the list is not greedy.

Luckily, the number of places needing such tweaks is small. There are a few places where trailing comma support was warranted too technically difficult and/or not worth the trouble - they are all documented below.

Each section below includes an approximate line number, simply to help reviewers find the location easier, as it is a big file. I've not consolidated all of the test cases into a single regression test, but am open to the idea (I'm not convinced it is needed).

Cheers,
Greg


=======
* utility_option_list

Status: fully supported

Line: 1160

Used by: CHECKPOINT, REINDEX, CLUSTER, VACUUM, ANALYZE, EXPLAIN

Test cases (should fail on head, but work with this patch):

  EXPLAIN (verbose, format yaml, ) select 1;

  ANALYZE (skip_locked,verbose,) pg_class;


=======
* var_list

Status: not supported

Line: 1854

Used by generic_set, but there a lot of shift/reduce conflicts. For example:

alter function foo() set search_path = public, abc immutable;

alter function foo() set search_path = public, abc, immutable;

If we allow trailing commas, would immutable be the name of the schema or the SET modifier?

In theory we could create a new list to allow trailing commas on simpler items, e.g.:

alter system set log_destination = stderr,csvlog,;

But I feel like this is going too far, so I left this one alone.


=======
* alter_table_cmds

Status: fully supported

Line: 2386

Used by ALTER TABLE to support multiple actions.
Also ALTER INDEX, ALTER SEQUENCE, ALTER VIEW, ALTER MATERIALIZED VIEW, ALTER FOREIGN TABLE

Test case:

  CREATE TABLE t(id int);
  ALTER TABLE t ADD foo INT, ADD bar INT,;


=======
* reloption_list

Status: fully supported

Line: 3192

Used by: reloptions and opt_reloptions to support SET() items for ALTER TABLE,
CREATE INDEX, CREATE VIEW, and a few others.

Test cases:

  ALTER TABLE foo SET (fillfactor=50,);

  CREATE INDEX i1 ON t(id) WITH (fillfactor=50, deduplicate_items=off,);


=======
* hash_partbound

Status: fully supported

Line: 3363

Used by: partitioning FOR VALUES WITH ()

Test cases:

  CREATE TABLE p(id INT) PARTITION BY hash(id);

  CREATE TABLE c PARTITION OF p FOR VALUES WITH (remainder 1, modulus 4,);


=======
* alter_type_cmds

Status: fully supported

Line: 3394

Used by: ALTER TYPE foo xxx

TEST CASE:

  CREATE TYPE mytype AS (id int);
  ALTER TYPE mytype ADD attribute id2 int, ADD attribute id3 int, ;


=======
* copy_generic_opt_list

Line: 3678

Used by: COPY ... WITH ()

Test case:

  COPY pg_language(oid,lanname) TO stdout WITH (header, format csv,);


=======
* copy_generic_opt_arg_list

Status: fully supported

Line: 3705

Used by: COPY WITH ( force_quote( [, ...] ) )

Test case:

  COPY pg_language(oid, lanname) TO stdout WITH (format csv, force_quote(oid,lanname, ) );


=======
* TableElementList

Status: fully supported

Line: 3898

Test case:

  CREATE TEMP TABLE foo(id int,);


=======
* TypedTableElementList

Status: fully supported

Line: 3908

Test case:

  CREATE TYPE abc AS (id int, id2 int);
  CREATE TABLE mytype OF abc (id default 1, id2 default 3,);


=======
* columnList

Status: partially supported

Line: 4542

In order:

1. UNIQUE constraints and PRIMARY KEY

Needed to modify opt_without_overlaps to add the optional comma directly:

opt_without_overlaps:
   ','                    { $$ = false; }
   | WITHOUT OVERLAPS     { $$ = true; }
   | ',' WITHOUT OVERLAPS { $$ = true; }
   | /*EMPTY*/            { $$ = false; }
   ;

Test case:

  CREATE TEMP TABLE t (c1 int4range, c2 int4range);
  ALTER TABLE t ADD CONSTRAINT test1 UNIQUE (c1, c2);
  ALTER TABLE t ADD CONSTRAINT test2 UNIQUE (c1, c2,);
  ALTER TABLE t ADD CONSTRAINT test3 UNIQUE (c1, c2 WITHOUT OVERLAPS);
  ALTER TABLE t ADD CONSTRAINT test4 UNIQUE (c1, c2, WITHOUT OVERLAPS);
  ALTER TABLE t ADD PRIMARY KEY (c1, c2, );


2. FOREIGN KEY

As above, but we modify optionalPeriodName to allow a leading comma.

Also add %nonassoc PERIOD

optionalPeriodName:
   ','                      { $$ = NULL; }
   | PERIOD columnElem      { $$ = $3; }
   | ',' PERIOD columnElem  { $$ = $3; }
   | /*EMPTY*/              { $$ = NULL; }
  ;

Test case:

  CREATE TEMP TABLE t (id INT PRIMARY KEY);
  CREATE TEMP TABLE t2 (id INT,
    CONSTRAINT tempo1 FOREIGN KEY(id,) REFERENCES t (id,));

  CREATE temp TABLE t3 (id INT4RANGE, valid_at DATERANGE,
    CONSTRAINT t3_pk PRIMARY KEY (id, valid_at, WITHOUT OVERLAPS));

  CREATE temp TABLE t4 (id INT4RANGE, valid_at DATERANGE,
    CONSTRAINT t4_fk FOREIGN KEY (id, PERIOD valid_at)
    REFERENCES t3 (id, PERIOD valid_at));

3. opt_column_list

Many sub-items for this

3.1 CopyStmt

Test case:

  COPY pg_language (lanowner,) TO stdout;


3.2 copy_opt_item FORCE_QUOTE

Test case:

   COPY pg_language (lanowner) TO stdout WITH (format csv, FORCE_QUOTE (lanowner,) );


3.3 copy_opt_item FORCE_NULL

Test case:

  CREATE TEMP TABLE T (foo TEXT);
  COPY t FROM program 'echo abc' WITH (format csv, FORCE_NULL (foo, ) );

3.4 ColConstraintElem REFERENCES

Test case:

  CREATE TEMP TABLE t (id INT PRIMARY KEY);
  CREATE TEMP TABLE t2 (id INT CONSTRAINT tempo REFERENCES t (id,));

3.5 key_action (e.g. SET NULL)

Test case:

  CREATE TEMP TABLE t (id INT PRIMARY KEY);
  CREATE TEMP TABLE t2 (id INT CONSTRAINT tempo REFERENCES t (id) ON DELETE SET NULL (id,) );

3.6 create_as_target

Test case:

  CREATE TEMP TABLE t (id,) AS select 123;

3.7 create_mv_target:

Test case:

  CREATE MATERIALIZED VIEW t(id,) AS select 123;
  DROP MATERIALIZED VIEW t;

3.8 privileges

Test case:

  CREATE TEMP TABLE t (id INT);
  GRANT SELECT (id, ) ON TABLE t TO public;

3.9 PublicationObjSpec

Test case:

  CREATE TABLE t(id INT);
  CREATE TABLE t2(id INT);
  CREATE PUBLICATION p FOR TABLE t(id,);
  DROP PUBLICATION p;
  CREATE PUBLICATION p FOR TABLE t *, ONLY t2 (id,);
  DROP PUBLICATION p;
  DROP TABLE t;

3.10 ViewStmt:

Test case:

  CREATE TEMP VIEW t (id, ) AS select 123;

4. opt_column_and_period_list:  '(' columnList optionalPeriodName ')'

Test case: see #3 "FOREIGN KEY" above

5. opt_c_include: INCLUDE '(' columnListOptionalComma ')'

Test case:

  CREATE TEMP TABLE t (id INT, id2 INT);
  ALTER TABLE t ADD CONSTRAINT u1 UNIQUE (id) INCLUDE (id2,);

6. TriggerOneEvent

Test case:

  CREATE TEMP TABLE t (id int, id2 int);
  CREATE FUNCTION f() returns trigger language plpgsql as 'begin return new; end';
  CREATE TRIGGER tr1 after UPDATE OF id, ON t for each row execute function f();
  drop function f() cascade;


7. privileges for columns

Test case:

  CREATE TEMP TABLE t(id int);
  GRANT ALL (id, ) on table t to public;


8. ViewStmt

Test case:

  CREATE recursive VIEW v (id,) as select 123;


9. opt_search_clause

Test case:

  WITH RECURSIVE x AS (select 123 as id union all select * from x)
    SEARCH DEPTH FIRST BY id, SET foo select * from x limit 1;

Also added a "depth_or_breadth" to refactor the opt_search_clause first


10. opt_cycle_clause

Test case:

  WITH RECURSIVE x AS (select 123 as id union all select * from x)
    CYCLE id, SET id2 to 3 default 1 using id3 select * from x limit 1;


End of columnList!


=======
* ExclusionConstraintList

Status: fully supported

Line: 4585

Used by: EXCLUDE

Done by: adding ExclusionConstraintListItems

Test case:

  CREATE TEMP TABLE tt(id int, EXCLUDE (id WITH =), );


=======
* part_params

Status: fully supported

Line: 4739

Used by: PartitionSpect etc. - only when creating partitioned tables

Done by: adding part_params_items

Test case:

  CREATE TEMP TABLE t(id int) PARTITION BY hash (id,);


=======
* stats_params

Status: fully supported

Line: 4859

Used by: create statistics

Done by: adding stats_params_items

Test case:

  CREATE TEMP TABLE t1(id int, id2 int);
  CREATE STATISTICS ON id, id2, FROM t1;


=======
* NumericOnly_list

Status: not supported

Line; 5188

Only used to set permissions on large objects, not worth the effort


=======
* generic_option_list

Status: fully supported

Line: 5628

Used by: create_generic_options for FDW items, e.g.
  CREATE FOREIGN DATA WRAPPER
  CREATE SERVER
  CREATE FOREIGN TABLE
  IMPORT FOREIGN SCHEMA
  CREATE USER MAPPING

Done by: adding generic_option_list_items

Test case:

  CREATE EXTENSION if not exists postgres_fdw;
  DROP SERVER if exists testserver CASCADE;
  CREATE SERVER testserver foreign data wrapper postgres_fdw OPTIONS (dbname 'foo',);
  CREATE USER MAPPING for postgres SERVER testserver OPTIONS (user 'alice', );
  CREATE FOREIGN TABLE foo(c1 int) SERVER testserver OPTIONS (schema_name 'bob',);
  DROP SERVER testserver CASCADE;


=======
* alter_generic_option_list

Status: fully supported

Line: 5644

Used by: similar to above, but modifying FDW-related objects

Done by: adding alter_generic_option_list_items

Test case:

  CREATE EXTENSION if not exists postgres_fdw;
  DROP SERVER if exists testserver CASCADE;
  CREATE SERVER testserver foreign data wrapper postgres_fdw OPTIONS (dbname 'foo');

  ALTER SERVER testserver OPTIONS (SET dbname 'foo2', );

  DROP SERVER testserver CASCADE;

=======
* TriggerFuncArgs

Status: fully supported

Line: 6329

Used by: arguments to functions, naturally

Done by: adding TriggerFuncArgsItems

Test case:

  CREATE TEMP TABLE t (id int);
  DROP FUNCTION if exists footrig() CASCADE;
  CREATE FUNCTION footrig() returns trigger language plpgsql as 'begin return new; end';
  CREATE TRIGGER tr after insert on t for each row execute function footrig(1,2,);


=======
* event_trigger_value_list:

Line: 6438

Status: fully supported

Used by: tags for event trigger creations

Done by: adding event_trigger_value_list_items

Test case:

  DROP FUNCTION if exists foo() CASCADE;
  CREATE FUNCTION foo() returns event_trigger language plpgsql as 'begin return;end';
  CREATE EVENT TRIGGER tfoo on sql_drop when tag in ('DROP TABLE',) execute function foo();
  DROP FUNCTION foo() CASCADE;


=======
* def_list

Status: fully supported

Line: 6661

Used by:
  "definition":
    create aggregate, operator, collation, type, search *
    alter publication, alter subscription
    alter search dictionary
  "opt_definition":
    column constraint unique, primary key, exclude
    create publication, create subscription, alter subscription

Done by: adding def_list_items

Test case:

  DROP OPERATOR if exists === (date,date);
  CREATE OPERATOR === ( leftarg=date, rightarg=date, function=date_eq, );
  DROP OPERATOR === (date,date);

  CREATE TEMP TABLE t (id int, UNIQUE(id) WITH (fillfactor=42, ) );

  DROP PUBLICATION if exists foo;
  CREATE PUBLICATION foo WITH (publish = 'insert', );
  DROP PUBLICATION foo;


=======
* old_aggr_list

Status: not supported

Line: 6680

Extremely old syntax for CREATE AGGREGATE, no need to support this.


=======
* enum_val_list

Status: fully supported

Line: 6707

Used by: enums!

Done by: adding enum_val_list_items

Test case:

  CREATE TYPE fooe AS ENUM ('foo', 'bar', );
  DROP TYPE fooe;


=======
* opcast_item_list

Status: fully supported

Line: 6830

Used by: create operator class and alter operator family (add)

Done by: adding opcast_item_list_items

Test case:

  DROP OPERATOR CLASS if exists oc USING gist;
  CREATE OPERATOR CLASS oc FOR TYPE int USING gist AS function 1 pi(), ;
  DROP OPERATOR CLASS oc USING gist;

  DROP OPERATOR FAMILY if exists ofam USING gist;
  CREATE OPERATOR FAMILY ofam USING gist;
  ALTER OPERATOR FAMILY ofam USING gist ADD operator 1 = (int, int), ;
  DROP OPERATOR FAMILY ofam USING gist;


=======
* opclass_drop_list

Status: fully supported

Line: 6936

Used by: alter operator family (drop)

Done by: adding opclass_drop_list_items

Test case:

  DROP OPERATOR FAMILY if exists ofam USING gin;
  CREATE OPERATOR FAMILY ofam USING gin;
  ALTER OPERATOR FAMILY ofam USING gin ADD operator 1 = (int, int);
  ALTER OPERATOR FAMILY ofam USING gin DROP operator 1 (int, int), ;
  DROP OPERATOR FAMILY ofam USING gin;


=======
* any_name_list

Status: fully supported

Line: 7236

Used by: privileges, text search configuration, DROP

1. privilege_target

Test case:

  CREATE DOMAIN testd1 AS int;
  CREATE DOMAIN testd2 AS int;
  GRANT usage ON DOMAIN testd1, testd2, TO public;
  DROP DOMAIN testd1, testd2;


2. privilege_target

Test case:

  CREATE TEMP TABLE foo1 (id int);
  CREATE TEMP TABLE foo2 (id int);
  GRANT usage ON TYPE foo1, foo2, TO public;

3. AlterTSConfigurationStmt

Test case:

  CREATE TEXT SEARCH CONFIGURATION tsc ( copy = simple );
  ALTER TEXT SEARCH CONFIGURATION tsc ADD MAPPING FOR tag, blank,  WITH simple;
  DROP TEXT SEARCH CONFIGURATION tsc;

4. DropStmt

Test Case:

  CREATE TEMP TABLE foo(id int);
  DROP TABLE foo, cascade;

=======
* type_name_list

Status: fully supported

Line: 7251

Used by: multiple "type" items

Done by: adding type_name_list_items

  CREATE TEMP TABLE foo1 (id int);
  CREATE TYPE foo AS (id int);
  DROP TYPE foo, cascade;


=======
* privilege_list

Status: fully supported

Line: 7894

Used by: all grant and revoke variants

Done by: adding privilege_list_items

Test case:

  CREATE TEMP TABLE t1 (id int);
  GRANT select, insert, ON t1 TO public;
  REVOKE select, insert, ON t1 FROM public;


=======
* parameter_name_list

Status: fully supported

Line: 7940

Used by: permissions on parameters

Done by: adding parameter_name_list_items

Test case:

  GRANT SET ON PARAMETER work_mem, TO public;


=======
* grantee_list

Status: fully supported

Line: 8158

Used by: GRANT to a list of roles

Done by: adding grantee_list_items, %nonassoc GRANTED

Test case:

  CREATE USER alice;
  CREATE TEMP TABLE t1 (id int);
  GRANT select ON TABLE t1 TO alice, alice,;
  GRANT select ON TABLE t1 TO alice, alice, WITH GRANT OPTION;
  GRANT select ON TABLE t1 TO alice, alice, GRANTED BY current_user;

Because GRANT has two optional items at the end of it:
opt_grant_grant_option opt_granted_by

We need to ensure that 'WITH' and 'GRANTED' have some extra stickiness. The former
already has it, but GRANTED was added to the %nonassoc list around line 898


=======
* grant_role_opt_list

Status: fully supported

Line: 8235

Used by: options when adding one role to another

Done by: adding grant_role_opt_list_items, %nonassoc GRANTED

Test case:

  CREATE USER alice;
  CREATE ROLE commarole;
  GRANT commarole TO alice WITH admin true, inherit false,;
  GRANT commarole TO alice WITH admin option, inherit true, GRANTED BY current_user;


=======
* index_params

Status: fully supported

Line: 8439

Used by: list of columns when creating an index

Done by: adding index_params_items

Test case:

  CREATE TEMP TABLE t (id int);
  CREATE INDEX ti1 ON t USING btree (id,);


=======
* index_including_params

Status: fully supported

Line: 8502

Used by: list of columns for covering indexes

Done by: adding index_including_params_items

Test case:

  CREATE TEMP TABLE t (id int, email text);
  CREATE INDEX ti1 ON t (id) INCLUDE (email, );


=======
* func_args_list

Status: fully supported

Line: 8603

Used by: certain places where we need to reference a function by its args

Done by: adding func_args_list_items

Test case:

  CREATE FUNCTION commatest(int) returns int language sql as 'select 1';
  COMMENT ON FUNCTION commatest(int,) IS 'Welcome, extra commas!';
  DROP FUNCTION commatest(int, );


=======
* function_with_argtypes_list

Status: fully supported

Line: 8613

Used by: dropping multiple functions at once, assigning privs to multiples

Done by: adding function_with_argtypes_list_items, %nonassoc CASCADE RESTRICT

Test case:

  CREATE FUNCTION commatest(int) returns int language sql as 'select 1';
  DROP FUNCTION commatest(int),;
  DROP FUNCTION if exists commatest(int), CASCADE;


=======
* func_args_with_defaults_list:

Status: fully supported

Line: 8665

Used by: list of args when doing a create function only

Done by: adding func_args_with_defaults_list_items

Test case:

  CREATE FUNCTION commatest(int, int, ) returns int language sql as ' select 42 ';


=======
* aggr_args_list

Status: fully supported

Line: 8864

Used by: aggregate declaration

Done by: adding aggr_args_list_items

Test case:

  CREATE AGGREGATE foo (int, ) (sfunc=gcd, stype=int);
  DROP AGGREGATE foo(int,);


=======
* aggregate_with_argtypes_list

Status: fully supported

Line: 8881

Used by: operations on multiple aggregates at once

Done by: adding aggregate_with_argtypes_list_items, %nonassoc CASCADE RESTRICT

Test case:

  CREATE AGGREGATE foo1 (int) (sfunc=gcd, stype=int);
  CREATE AGGREGATE foo2 (int) (sfunc=gcd, stype=int);
  DROP AGGREGATE foo1(int), foo2(int), ;


=======
* func_as

Status: not supported

Line: 9044

Putting this in for completelness, no trailing comma support is needed, as there are only two items


=======
* transform_type_list

Status: fully supported

Line: 9053

Used by: functions that transform types

Done by: adding transform_type_list_items

Test case:

  CREATE TRANSFORM FOR int LANGUAGE sql (from sql with function time_support(internal) );
  CREATE FUNCTION foo() returns int language sql
    transform for type int, for type int4, as 'select 1';
  DROP TRANSFORM FOR int language sql cascade;


=======
* table_func_column_list

Status: fully supported

Line: 9076

Used by: list of columns for a table-returning function

Done by: adding table_func_column_list_items

Test case:

  CREATE FUNCTION foo() RETURNS table(x int,) language sql as 'select 1';
  DROP FUNCTION foo();


=======
* oper_argtypes

Status: not supported

Line: 9323

Used by: arguments to operators ("left" and "right")

No support for trailing commas needed, unless we want to get really pedantic


=======
* operator_with_argtypes_list

Status: fully supported

Line: 9293

Used by: dropping multiple operators at once

Done by: adding operator_with_argtypes_list_items, %nonassoc CASCADE RESTRICT

Test case:

  CREATE OPERATOR === ( leftarg=date, rightarg=date, function=date_eq);
  CREATE OPERATOR ==== ( leftarg=date, rightarg=date, function=date_eq);
  DROP OPERATOR === (date,date), ==== (date, date), ;


=======
* operator_def_list

Status: fully supported

Line: 10521

Used by: modifying an existing operator

Done by: adding operator_def_list_items

Test case:

  CREATE OPERATOR === ( leftarg=date, rightarg=date, function=date_eq);
  ALTER OPERATOR === (date, date) SET (hashes, );
  DROP OPERATOR === (date, date);


=======
* pub_obj_list

Status: fully supported

Line: 10929

Used by: create and alter publications

Done by: adding pub_obj_list_items

Test case:

  CREATE TABLE foo1 (id int);
  CREATE TABLE foo2 (id int);
  CREATE PUBLICATION p FOR TABLE foo1, table foo2, ;
  DROP PUBLICATION p;
  DROP TABLE foo1, foo2;


=======
* pub_obj_type_list

Status: fully supported

Line: 10950

Used by: create publication (as of v19 only)

Done by: adding pub_obj_type_list_items (also cleaned up nearby indentation issue)

Test case:

  CREATE PUBLICATION p FOR ALL TABLES, ALL SEQUENCES;
  DROP PUBLICATION p;


=======
* notify_payload

Status: not supported

Line: 11335

Used by: notify with a channel plus a payload

Seems not needed, given that this is not really a list of items per se, just an optional payload.


=======
* transaction_mode_list

Status: fully supported

Line: 11462

Used by: set transaction

Done by: adding transaction_mode_list_items

Test case:

  BEGIN WORK deferrable, isolation level serializable, ; ROLLBACK;


=======
* drop_option_list

Status: fully supported

Line: 11756

Used by: drop database only

Done by: adding drop_option_list_items

Test case:

  DROP DATABASE bob with (force,force,);

Only option right now is "force" but this is for future-proofing things.


=======
* vacuum_relation_list

Status: fully supported

Line: 12199

Used by: vacuum multiple things at once

Done by: adding vacuum_relation_list_items

Test case:

  VACUUM pg_am, pg_proc, ;


=======
* insert_column_list

Status: fully supported

Line: 12480

Used by: your basic insert statement

Done by: adding insert_column_list_items

Test case:

  CREATE TEMP TABLE t (id int);
  INSERT INTO t(id,) VALUES (1);


=======
* returning_options

Status: not supported

Line: 12641

Used by: insert that uses RETURNING WITH

Does not seem worth it, as we only support OLD and NEW


=======
* set_clause_list

Status: fully supported

Line: 12686

Used by: UPDATE .. SET

Done by: adding set_clause_list_items

Test case:

  CREATE TEMP TABLE t (id int);
  UPDATE t SET id = 1, WHERE id <> 0;


=======
* set_target_list

Status: fully supported

Line: 12731

Used by: SET a bunch of things at once inside parens

Done by: adding set_target_list_items

Test case:

  CREATE TEMP TABLE t (id int);
  UPDATE t SET (id,) = ROW(1);


=======
* cte_list

Status: not supported

Line: 13287

Used by: chaining multiple CTEs together

This one is not likely to be supported, there is way too much ambiguity.


=======
* sortby_list

Status: partially supported

Line: 13407

Used by: ORDER BY and json_array_aggregate_order_by_clause_opt

Done by: adding sortby_list_items and a new sortby_list_no_trailing_comma

Test case:

  SELECT * FROM pg_language ORDER BY 1,2,3, LIMIT 1;

This one needed to be split into two versions, as the "normal" ORDER BY did
just fine with adding a comma at the end of the list. The json aggregate
however, spit up shift/reduce errors. That's a rather niche usage of
ORDER BY, so at the end of the day, I decided to support the 99% usage and
leave the json one alone.


=======
* group_by_list

Status: fully supported

Line: 13782

Used by: group by, of course

Done by: adding group_by_list_items

Test case:

  SELECT datname, count(*) FROM pg_database GROUP BY 1, ;


=======
* from_list

Status: fully supported

Line: 13786

Used by: list of tables in a from clause

Done by: adding from_list_items

Test case:

  SELECT count(*) FROM pg_am, pg_proc, ;


=======
* relation_expr_list

Status: fully supported

Line: 14144

Used by: import foreign schema, truncate tables, lock tables

Done by: adding relation_expr_list_items, %nonassoc RESTART CONTINUE_P

Not completely happy about having to add two more keywords just to
support TRUNCATE, but TRUNCATE is a rather important command, so
I think it is worth it.

Test case:

  CREATE TEMP TABLE t1 (id int);
  CREATE TEMP TABLE t2 (id int);
  TRUNCATE TABLE t1, t2, ;
  TRUNCATE TABLE t1, t2, CONTINUE IDENTITY;
  LOCK TABLE pg_am, pg_proc, NOWAIT;

There is a little bit of ambiguity there as there is a very weak use case
for doing what that LOCK TABLE above used to do: apply the default lock
mode to three tables, one of which is named "nowait". But "nowait" is
a dumb name for a table, so I think caveat emptor applies here. The same
thing applies to some of the other examples, e.g. having a table named "CASCADE")


=======
* rowsfrom_list

Status: fully supported

Line: 14243

Used by: rows from a list - see below

Done by: adding rowsfrom_list_items

Test case:

  SELECT * FROM ROWS FROM ( abs(1), abs(2), );


=======
* TableFuncElementList

Status: fully supported

Line: 14283

Used by: aliases for functions with declared list of columns

Done by: adding TableFuncElementListItems

Test case:

  SELECT * FROM jsonb_to_record('{"foo": 10}') AS x (foo int, );


=======
* xmltable_column_list

Status: fully supported

Line: 14343

Used by: The weird xmltable function

Done by: adding xmltable_column_list_items

Test case:

  WITH xmldata(data) AS (VALUES
    ('<aa xmlns="https://xmlsux"><item foo="42" /></aa>'::xml))
  SELECT xmltable.* FROM XMLTABLE(xmlnamespaces('https://xmlsux' AS x),
    '/x:aa/x:item' passing (select data from xmldata)
     COLUMNS foo int path '@foo', );


=======
* xml_namespace_list

Status: fully supported

Line: 14460

Used by: xml stuff

Done by: adding xml_namespace_list_items

Test case:

  WITH xmldata(data) AS (VALUES
    ('<aa xmlns="https://xmlsux"><item foo="42" /></aa>'::xml))
  SELECT xmltable.* FROM XMLTABLE(xmlnamespaces('https://xmlsux' AS x, ),
    '/x:aa/x:item' passing (select data from xmldata)
     COLUMNS foo int path '@foo' );


=======
* json_table_column_definition_list:

Status: fully supported

Line: 14518

Used by:

Done by: adding json_table_column_definition_list_items

Test case:

  SELECT * from json_table('{"id": 123}', '$[*]' columns (id int path '$.id',));


=======
* xml_attribute_list

Status: fully supported

Line: 16480

Used by: XML stuff

Done by: adding xml_attribute_list_items

Test case:

  SELECT xmlforest(1 as is, );


=======
* window_definition_list

Status: fully supported

Line: 16832

Used by:

Done by:

Test case:

  SELECT 1 from pg_database window foo as (partition by oid), ;


=======
* expr_list

Status: partially supported

Line: 16903

Used by: lots of things - will break down each one

Done by: adding expr_list_items, plus expr_list_no_trailing_comma

1. list and range partition definitions

Supported. Test case:

  CREATE TEMP TABLE t1 (id int) partition by list (id);
  CREATE TEMP TABLE t2 partition of t1 for values in (1,2,);
  CREATE TEMP TABLE t3 (id int, id2 int) partition by range (id, id2);
  CREATE TEMP TABLE t4 partition of t3 for values from (1,2,) to (6,7,);

2. execute parameters

Supported. Test case:

  PREPARE foo as select $1;
  EXECUTE foo (1, );
  DEALLOCATE foo;

3. merge values

Supported. Test case:

  CREATE TEMP TABLE t (id int);
  MERGE INTO t USING (values (1)) on (true) when not matched then insert (id) values (42,);

4. distinct on

Supported. Test case:

  SELECT distinct on (prolang,) prolang from pg_proc;

5. rollup and cube

Supported. Test case:

  SELECT relkind, relnamespace, count(*) from pg_class group by rollup(1,2,);
  SELECT relkind, relnamespace, count(*) from pg_class group by cube(1,2,);

6. values

Supported. Test case:

  SELECT * FROM (values(1,2,), (3,4,) );

7. tablesample

Supported. Test case:

  SELECT relname from pg_class tablesample bernoulli (1,);

Yes, I know bernoulli only accepts a single arg anyway.

8. generic type modifiers

Supported. Test case:

  SELECT 123::numeric(10, );

9. bit type with a length

Supported. Test case:

  SELECT 42::bit(8,);

10. general expression foo in (list) and not in (list)

Supported. Test case:

  SELECT 1 IN (1,2,3,);
  SELECT 1 NOT IN (1,2,3,);

11. grouping

Supported. Test case:

  SELECT relkind, GROUPING(relkind,), count(*) from pg_class group by 1;

12. coalesce, greatest, least

Supported. Test case:

  SELECT COALESCE(1,2, );
  SELECT GREATEST(42, 24,);
  SELECT LEAST(42, 24,);

13. xmlconcat, xmlelement

Supported. Test case:

  SELECT XMLCONCAT('1'::xml, '2'::xml, );
  SELECT XMLELEMENT(name foo, 'fizz', 'buzz',);

14. partition by inside a window

Supported. Test case:

  SELECT lag(oid) over(PARTITION BY 1,2,) from pg_am;

15. ROW

PARTIALLY supported. ROW is a tricky case. In the end, I was only able to
reliably get the explicit_row to work with a hard-coded comma variant. On the
other hand, that's a pretty common form, so I'm happy overall.

Test case:

  SELECT ROW(1,2,3,);
  /* Still fails: SELECT (1,2,3,); */

16. array lists

Supported. Test case:

  SELECT ARRAY[1,2,3,];

17. trim list

Supported. Test case:

  SELECT TRIM(both from 'abba', 'a', );

Not particularly useful, but it's kinda built-in with the expr_list.

This is the end of expr_list!


=======
* func_arg_list

Not to be confused with func_args_list, way back around line 8600!

Status: partially supported

Line: 17036

Used by: arguments to functions

Done by: func_arg_list_items, func_arg_list_no_trailing_comma

Test case:

  /* Works for simple variants: */
  CREATE PROCEDURE foo(int) language sql as 'select 1';
  CALL foo(1,);
  CALL foo(ALL 1,);
  DROP PROCEDURE foo(int);

This is only partly supported, due to the weird way in which we can call functions,
but I think the main cases are covered well enough.

=======
* type_list

Status: fully supported

Line: 16954

Used by: prepare arguments

Done by: adding type_list_items

Test case:

  PREPARE foo(int,) AS select $1;


=======
* array_expr_list

Status: fully supported

Line: 16972

Used by: multiple array items

Done by: adding array_expr_list_items

Test case:

  SELECT ARRAY[ [1],[2], ];


=======
* json_arguments

Status: not supported

Line: 17334

Test case:

  SELECT JSON_VALUE(jsonb '[]', '$' PASSING 1 as x, 2 as y,);

Only used in a relatively obscure command; lots of shift/reduce conflicts; not worth the effort


=======
* json_name_and_value_list

Status: not supported

Line: 17496

Test case:

  SELECT json_object(1:2,3:4,);

As above, too many shift/reduce conflicts as there are many post-comma opttions.

=======
* json_value_expr_list:

Status: not supported

Line: 17529

Test case:

  SELECT json_array(1,2,3,);

Continuing our json trend, we cannot support this one either


=======
* target_list

Status: fully supported

Line: 17456

Used by: select a list of things

Done by: adding target_list_items

Test case:

  SELECT 1,2,3, ;


=======
* qualified_name_list

Status: fully supported

Line: 17507

Used by:

Done by: adding qualified_name_list_items, %nonassoc DEFERRED IMMEDIATE SKIP NOWAIT

This important clause was a little tricky and requires some %nonassoc magic

1. set constraints (see ConstraintsSetStmt)

The full syntax is:
  SET CONSTRAINTS constraints_set_list constraints_set_mode

where contraints_set_list contains our qualified name list, and the mode can be
DEFERRED or IMMEDIATE. So we add those two as %nonassoc

Test case:

  CREATE TEMP TABLE foo(x int, y int);
  ALTER TABLE foo add constraint xplus check(x > 0), add constraint yplus check(y > 0);
  BEGIN;
  SET CONSTRAINTS xplus, yplus, immediate;
  ROLLBACK;


2. table inheritance list (see CreateStmt)

Test case:

  CREATE TEMP TABLE foo() INHERITS (pg_proc, );

3. grant/revoke list of items (see privilege_target)

Test case:

  GRANT select on table pg_class, to public;

4. Locking multiple objects (see for_locking_item)

Test case:

  SELECT 1 from pg_database, pg_am for update of pg_database nowait limit 1;
  SELECT 1 from pg_database, pg_am for update of pg_database, nowait limit 1;
  SELECT 1 from pg_database, pg_am for update of pg_database, pg_am nowait limit 1;
  SELECT 1 from pg_database, pg_am for update of pg_database, pg_am, nowait limit 1;

Since the list here is followed by some optional items, we needed
to add those to %nonassoc: SKIP NOWAIT


=======
* name_list

Status: fully supported

Line: 17701

Used by: tons of things that need, well, a list of names

Done by: adding name_list_items, %nonassoc RESTRICT CASCADE REVOKE REPLACE

Test case:

  GRANT USAGE on schema public, TO public;


=======
* role_list

Status: fully supported

Line: 17753

Used by: list of roles (e.g. to grant multiple roles at once)

Done by: adding role_list_items, plus large %nonassoc list:
  CONNECTION INHERIT ENCRYPTED ADMIN PASSWORD ROLE SYSID UNENCRYPTED VALID

Test case:

  GRANT pg_monitor TO alice, ;

I don't like adding so many %nonassoc for just this one simple case, but I think
it is warranted as a list of role is pretty common.


That's the end of the list! Congratulations on reaching this far, even if you
simply scrolled to the bottom without reading everything! :) The total number of
changes was not too bad, just spread out a lot:

$ git log --oneline -1 --shortstat
95a599b62a7 (HEAD -> comma,commma,commma,comma,chameleon, master) Provide support for trailing commas, where possible and practical.
 1 file changed, 564 insertions(+), 305 deletions(-)


Attachment

pgsql-hackers by date:

Previous
From: Sami Imseih
Date:
Subject: Re: Refactor query normalization into core query jumbling
Next
From: Tom Lane
Date:
Subject: Re: [PATCH] Provide support for trailing commas