Обсуждение: [PATCH] Provide support for trailing commas
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(-)
(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(-)
Вложения
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
ú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
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
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
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.
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.
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.
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
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.
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