Обсуждение: CREATE/ALTER PUBLICATION improvements for syntax synopsis

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

CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Peter Smith
Дата:
During some recent reviews in this area, I noticed both CREATE/ALTER
PUBLICATION synopses say:

----------
where publication_object is one of:

    TABLE [ ONLY ] table_name [ * ] [ ( column_name [, ... ] ) ] [
WHERE ( expression ) ] [, ... ]
    TABLES IN SCHEMA { schema_name | CURRENT_SCHEMA } [, ... ]
----------

IMO it would be better to include another pair of { } preceding the
TABLE ellipsis:
a) for consistency with the second (TABLES IN SCHEMA) case
b) to help remove ambiguity, what part of the syntax the TABLE ellipsis is for

e.g.
----------
    TABLE { [ ONLY ] table_name [ * ] [ ( column_name [, ... ] ) ] [
WHERE ( expression ) ] } [, ... ]
    TABLES IN SCHEMA { schema_name | CURRENT_SCHEMA } [, ... ]
----------

I attached a v1 patch to do this.

Thoughts?

======
Kind Regards,
Peter Smith.
Fujitsu Australia
Вложения

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Peter Smith
Дата:
Added a CF entry [1] for this.

======
[1]  https://commitfest.postgresql.org/patch/6062/

Kind Regards,
Peter Smith.
Fujitsu Australia

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Peter Smith
Дата:
A rebase was needed. Here is patch v2.

======
Kind Regards,
Peter Smith.
Fujitsu Australia
Вложения

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Peter Smith
Дата:
A rebase was needed. Here is patch v3.

======
Kind Regards,
Peter Smith.
Fujitsu Australia
Вложения

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Fujii Masao
Дата:
On Fri, Nov 14, 2025 at 10:23 AM Peter Smith <smithpb2250@gmail.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.

Regards,

--
Fujii Masao

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Chao Li
Дата:

> On Nov 14, 2025, at 15:47, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Fri, Nov 14, 2025 at 10:23 AM Peter Smith <smithpb2250@gmail.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 … ]
```

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Peter Smith
Дата:
On Fri, Nov 14, 2025 at 7:02 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On Nov 14, 2025, at 15:47, Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Fri, Nov 14, 2025 at 10:23 AM Peter Smith <smithpb2250@gmail.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
Вложения

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Chao Li
Дата:

> On Nov 17, 2025, at 08:30, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Fri, Nov 14, 2025 at 7:02 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Nov 14, 2025, at 15:47, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>
>>> On Fri, Nov 14, 2025 at 10:23 AM Peter Smith <smithpb2250@gmail.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).
>

I don’t like this renaming. Or at lease don’t use plural.

> #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"
>

I think PG doc usually place the most common form first. For publications, TABLE is used far more than TABLES IN
SCHEMA.For example, if you look at https://www.postgresql.org/docs/18/sql-grant.html, it doesn’t follow the
general->detailedrule. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis

От
Chao Li
Дата:

> On Nov 17, 2025, at 14:48, Chao Li <li.evan.chao@gmail.com> wrote:
>
>> #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).
>>
>
> I don’t like this renaming. Or at lease don’t use plural.

I incidentally clicked on “Send” too quick. I’d add a few words on this.

Here, “all_public_object” refers to “ALL TABLES” or “ALL SEQUENCES”. In this context, the thing is “ALL” instead of
multipleobjects, that’s why I don’t like the renaming, or at lease the new name should not use plural. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Чтобы сделать работу с сайтом удобнее, мы используем cookie и аналитический сервис «Яндекс.Метрика». Продолжая пользоваться сайтом, вы соглашаетесь с их использованием.