On Tue, Jun 9, 2026 at 12:12 PM Peter Smith <[email protected]> wrote:
>
> Hi Nisha,
>
> Thanks for following-up on this...
>
> The patch LGTM in general, but here are a few comments.
>

Thanks for the review.

> ======
>
> 1.
> +   <varlistentry>
> +    <term><literal>EXCEPT</literal></term>
> +    <listitem>
> +     <para>
> +      This clause specifies a list of tables to be excluded from the
> +      publication when used with <literal>SET ALL TABLES</literal>.  If
> +      <literal>EXCEPT</literal> is specified, the existing exclusion list is
> +      replaced with the specified tables.  If <literal>EXCEPT</literal> is
> +      omitted, any existing table exclusions are removed.  See
> +      <xref linkend="sql-createpublication-params-for-except-table"/> for
> +      details.
> +     </para>
> +    </listitem>
> +   </varlistentry>
>
> 1a.
> I think reversing the first sentence wording makes it simpler.
>
> SUGGESTION
> This clause can be used with <literal>SET ALL TABLES</literal> to
> specify a list of tables to be excluded from the publication.
>

I originally kept it consistent with the wording used in the CREATE
PUBLICATION doc, but I agree with your suggestion. Updated
accordingly.

> ~~~
>
> 1b.
> The term is written differently to how you wrote it in your other thread [1].
>
> Here, you write the term as style #1:
> <term><literal>EXCEPT</literal></term>
>
> But, in the other thread you wrote the same term as style #2:
> <term><literal>EXCEPT ( <replaceable
> class="parameter">except_table_object</replaceable> [, ... ]
> )</literal></term>
>
> I am not sure which way is better. If we are eventually going to put
> all variations of EXCEPT in the same place then maybe style #1 is
> best; OTOH, if we anticipate separate descriptions for the "EXCEPT
> (TABLE ...)" and "EXCEPT (SEQUENCE ...)" then maybe style #2 is best.
>

I think we should keep style #1, as it is consistent with the style
used in create_publication.sgml.

I'll also update my other thread to follow the same style so things
remain consistent until this patch is committed.

> ~~~
>
> 1c.
> The xref looks a bit strange because it renders like "See EXCEPT for
> details", but we are already looking at "EXCEPT", just not the same
> one. Perhaps it should be changed so it renders like "See CREATE
> PUBLICATION ... EXCEPT for details".
>

Makes sense. Fixed.
~~

Attached v2 patch.

--
Thanks,
Nisha

Attachment: v2-0001-doc-add-missing-EXCEPT-parameter-entry-in-ALTER-P.patch
Description: Binary data

Reply via email to