| From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis |
| Date: | 2025-11-17 00:30:47 |
| Message-ID: | CAHut+Pv2L-Xenugnf3ZhUOOUt+Nddf28yAYkO1aJvU4ti42ACw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Nov 14, 2025 at 7:02 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Nov 14, 2025, at 15:47, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> >
> > On Fri, Nov 14, 2025 at 10:23 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >>
> >> A rebase was needed. Here is patch v3.
> >
> > Thanks for the patch! LGTM.
> >
> > For example, in the CREATE PUBLICATION synopsis, the part that can be
> > repeated is "[ ONLY ] table_name ... [ WHERE ( expression ) ]" not just
> > the WHERE clause, so using curly brackets around that seems correct.
> >
>
> I disagree. {…} means “choose exactly one of the following alternatives”, but not for grouping for repetition.
>
> For example:
>
> ```
> GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | MAINTAIN }
> [, ...] | ALL [ PRIVILEGES ] }
> ON { [ TABLE ] table_name [, ...]
> | ALL TABLES IN SCHEMA schema_name [, ...] }
> TO role_specification [, ...] [ WITH GRANT OPTION ]
> [ GRANTED BY role_specification ]
> ```
>
> The two levels of {} are all for alternatives.
>
> So, I think the correct way is like:
>
> ```
> TABLE table_spec [, TABLE table_spec … ]
> ```
>
Fair point. I've changed v4 to use a syntax similar to your suggestion:
But, instead of "TABLE <table_spec> [, <table_spec> ...]", I am just
using "TABLE <table_spec> [,...]". Maybe it is not strictly correct,
but AFAICT it is consistent with how [,...] is used elsewhere in this
and other synopses.
e.g. we don't say "<column_name> [, <column_name> ...]"
~~~
Actually, I had a couple of other changes in the pipeline for the
synopsis that I was going to post as separate patches, but since this
has become a larger change, it is probably more appropriate to address
them all at once instead of churning the same synopsis multiple times.
So, now this patch makes the following 3 changes:
#1.
My original change, to fix the [, ...] grouping to remove ambiguity.
#2
Now renames "all_publication_object" to "publication_all_objects".
This is a simple name change that does not affect anything. I felt
everything ought to have the prefix of the object it belongs to (e.g.
"publication_name", "publication_parameter", "table_name",
"schema_name", column_name" all follow this rule, but prefix "all_"
was the odd-one-out).
#3
Rearranged the synopsis order from general to detailed. Again, there
is no functional difference; I just felt it was better to use the
natural logical order: e.g., "publication_all_objects" >
"publication_object"
======
Kind Regards,
Peter Smith.
Fujitsu Australia
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0001-Fix-synopsis.patch | application/octet-stream | 3.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexander Korotkov | 2025-11-17 00:34:36 | Re: Use opresulttype instead of calling SearchSysCache1() in match_orclause_to_indexcol() |
| Previous Message | Thomas Munro | 2025-11-16 23:54:44 | Re: Dead code in ps_status.c |