Обсуждение: [PATCH] Provide support for trailing commas

Поиск
Список
Период
Сортировка

[PATCH] Provide support for trailing commas

От
Greg Sabino Mullane
Дата:
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(-)


Вложения

Re: [PATCH] Provide support for trailing commas

От
Tom Lane
Дата:
Greg Sabino Mullane <htamfids@gmail.com> writes:
> 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,);

Did we not discuss and reject this very idea in the past?
A quick search finds [1][2], and there may be more threads.

> This is not as trivial a task as it seems, as those who have dabbled with
> our parser (or parsers in general) may suspect.

Indeed.  I think it carries substantial risk of creating large parsing
problems down the road, when we try to integrate whatever random
syntax the SQL committee comes up with next.  And frankly, I don't
agree that it'd be beneficial.

            regards, tom lane

[1] https://www.postgresql.org/message-id/flat/20140928114246.GA22150%40artax.karlin.mff.cuni.cz
[2] https://www.postgresql.org/message-id/flat/0368c60f-abe2-4f5f-972d-7cd1e6db2382%40gmail.com



Re: [PATCH] Provide support for trailing commas

От
Pavel Stehule
Дата:


út 6. 1. 2026 v 21:49 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Greg Sabino Mullane <htamfids@gmail.com> writes:
> 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,);

Did we not discuss and reject this very idea in the past?
A quick search finds [1][2], and there may be more threads.

> This is not as trivial a task as it seems, as those who have dabbled with
> our parser (or parsers in general) may suspect.

Indeed.  I think it carries substantial risk of creating large parsing
problems down the road, when we try to integrate whatever random
syntax the SQL committee comes up with next.  And frankly, I don't
agree that it'd be beneficial.

+1

Pavel
 

                        regards, tom lane

[1] https://www.postgresql.org/message-id/flat/20140928114246.GA22150%40artax.karlin.mff.cuni.cz
[2] https://www.postgresql.org/message-id/flat/0368c60f-abe2-4f5f-972d-7cd1e6db2382%40gmail.com


Re: [PATCH] Provide support for trailing commas

От
Greg Sabino Mullane
Дата:
On Tue, Jan 6, 2026 at 3:48 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Indeed.  I think it carries substantial risk of creating large parsing problems down the road, when we try to integrate whatever random syntax the SQL committee comes up with next.  And frankly, I don't agree that it'd be beneficial.

Hmmm, I was partly inspired to write this by your comment in the second link you referenced, in which you said:

> Yeah, a more principled approach would be to not special-case target
> lists, but to allow one (and only one) trailing comma everywhere:
> select, order by, group by, array constructors, row constructors,
> everything that looks like a function call, etc.

If it can be made to work everywhere, that would get my vote.

So, this is an attempt to be more comprehensive, which has been one of the major critiques of previous attempts to apply things to a small subset of the things we parse.

No, it's not yet in the SQL standard, but why not be more liberal in what we accept, and not have people invent various workaround hacks (e.g. dummy columns)? Because the parser eats the comma, there are no behavioral changes post-parsing.

Unsurprisingly, I also don't agree that new SQL committee changes will be that hard to support around some trailing comma support. The syntax moves slowly, and very few things will impact our comma-separated lists. The most likely things is additional modifiers e.g. RESTART IDENTITY but once we are handling one, we already have a framework in place to handle future ones.


Cheers,
Greg

Re: [PATCH] Provide support for trailing commas

От
David Rowley
Дата:
On Thu, 8 Jan 2026 at 03:07, Greg Sabino Mullane <htamfids@gmail.com> wrote:
> No, it's not yet in the SQL standard, but why not be more liberal in what we accept,

I believe the concern is that it could hinder us from being able to
implement new SQL standard features. Back in 8.4, "AS" became optional
in the SELECT list. I recall that it was a bit tricky to get around
some grammar ambiguities with that. There's some detail on that in the
comment added in e67867b26.

> and not have people invent various workaround hacks (e.g. dummy columns)? Because the parser eats the comma, there
areno behavioral changes post-parsing.
 

Why do people have to add dummy columns? Is this because these people
don't know how to program a check to see if they're on the first
iteration of a loop or not?

> Unsurprisingly, I also don't agree that new SQL committee changes will be that hard to support around some trailing
commasupport.
 

To me, that seems like a risky thing to disagree with. I expect you
don't have a vision of what the standard committee might dream up
infinitely far into the future.

Personally, I don't want this feature. I'd rather receive an error
message when I make silly mistakes in code that I write. If I have
code to dynamically build an SQL statement and I have a bug that
causes a variable that stores a column name to sometimes be an empty
string, with your patch, I'll get an error message in all cases unless
the column's position is last. I want the error at all times, not
sometimes.

David



Re: [PATCH] Provide support for trailing commas

От
Bruce Momjian
Дата:
On Tue, Jan  6, 2026 at 03:35:38PM -0500, Greg Sabino Mullane wrote:
> 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:

I am unclear why you bothered to create and work on such a large email
and patch rather than ask if this was a desired change;  see:

    https://wiki.postgresql.org/wiki/Todo#Development_Process

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.



Re: [PATCH] Provide support for trailing commas

От
Andres Freund
Дата:
Hi,

On January 7, 2026 11:44:29 PM EST, Bruce Momjian <bruce@momjian.us> wrote:
>On Tue, Jan  6, 2026 at 03:35:38PM -0500, Greg Sabino Mullane wrote:
>> 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:
>
>I am unclear why you bothered to create and work on such a large email
>and patch rather than ask if this was a desired change;  see:
>
>    https://wiki.postgresql.org/wiki/Todo#Development_Process

Meh. This kind of change really needs information about the implementation cost to be judged. The process described
thererarely is a good idea for anybody that has some community experience. 

Greetings,

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.



Re: [PATCH] Provide support for trailing commas

От
Bruce Momjian
Дата:
On Thu, Jan  8, 2026 at 12:51:14AM -0500, Andres Freund wrote:
> Hi, 
> 
> On January 7, 2026 11:44:29 PM EST, Bruce Momjian <bruce@momjian.us> wrote:
> >On Tue, Jan  6, 2026 at 03:35:38PM -0500, Greg Sabino Mullane wrote:
> >> 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:
> >
> >I am unclear why you bothered to create and work on such a large email
> >and patch rather than ask if this was a desired change;  see:
> >
> >    https://wiki.postgresql.org/wiki/Todo#Development_Process
> 
> Meh. This kind of change really needs information about the
> implementation cost to be judged. The process described there rarely is
> a good idea for anybody that has some community experience.

I disagree.  It was clear in this case that it was the concept that was
not wanted, rather than the implementation cost.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.



Re: [PATCH] Provide support for trailing commas

От
Andres Freund
Дата:
Hi,

On 2026-01-08 10:03:22 -0500, Bruce Momjian wrote:
> On Thu, Jan  8, 2026 at 12:51:14AM -0500, Andres Freund wrote:
> > Hi, 
> > 
> > On January 7, 2026 11:44:29 PM EST, Bruce Momjian <bruce@momjian.us> wrote:
> > >On Tue, Jan  6, 2026 at 03:35:38PM -0500, Greg Sabino Mullane wrote:
> > >> 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:
> > >
> > >I am unclear why you bothered to create and work on such a large email
> > >and patch rather than ask if this was a desired change;  see:
> > >
> > >    https://wiki.postgresql.org/wiki/Todo#Development_Process
> > 
> > Meh. This kind of change really needs information about the
> > implementation cost to be judged. The process described there rarely is
> > a good idea for anybody that has some community experience.
> 
> I disagree.  It was clear in this case that it was the concept that was
> not wanted, rather than the implementation cost.

I don't agree with that, fwiw. I it were easy to implement, I think we ought
to allow trailing commas in many places, most importantly the select list and
from list.

Greetings,

Andres Freund



Re: [PATCH] Provide support for trailing commas

От
Jelte Fennema-Nio
Дата:
On Thu, 8 Jan 2026 at 16:03, Bruce Momjian <bruce@momjian.us> wrote:
> I disagree.  It was clear in this case that it was the concept that was
> not wanted, rather than the implementation cost.

It definitely wasn't clear IMO. See the quote from Tom, that Greg
shared from that thread. Tom at that point said he'd be in favor, IF
it would be done everywhere. Which is exactly what Greg did.

Ofcourse Tom is allowed to change his opinion, but if I'd felt
strongly enough about this then I'd also have implemented this first
based on  the previous thread.

For the record, I'd very much like trailing commas to work. Just not
enough to put the time in to discuss them at length. But I'd
definitely like to not get errors thrown in my face every time I do a
trivial modification of a query, and I'd also like my git diffs not to
touch lines that logically have nothing to do with the change I'm
actually doing.



Re: [PATCH] Provide support for trailing commas

От
Bernd Helmle
Дата:
Am Donnerstag, dem 08.01.2026 um 10:53 +1300 schrieb David Rowley:
> Personally, I don't want this feature. I'd rather receive an error
> message when I make silly mistakes in code that I write. If I have
> code to dynamically build an SQL statement and I have a bug that
> causes a variable that stores a column name to sometimes be an empty
> string, with your patch, I'll get an error message in all cases
> unless
> the column's position is last. I want the error at all times, not
> sometimes.

This is exactly what i initially thought when reading through this
thread. Yes, people have their arguments, but it also feels like a big
hammer for a rather small issue, looking on what technical implications
are discussed and might hit us.

In other words, I never got a single complaint from people using
PostgreSQL in the last two decades for not allowing trailing commas in
our parser. And to be honest, i never thought about this before Greg's
proposal ...


--
Thanks,
    Bernd