Re: Support EXCEPT for ALL SEQUENCES publications

Hi,

On Fri, Jul 3, 2026 at 3:18 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>
> On Fri, 3 Jul 2026 at 11:24, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > On Thu, Jul 2, 2026 at 11:53 AM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
> > >
> > > Please find the updated v17 patch attached.
> > >
> >
> > Thank you for the updated patches. I see that the following command
> > works with your patch:
> >
> > CREATE PUBLICATION combined_seq_pub_1
> > FOR ALL SEQUENCES
> > EXCEPT (SEQUENCE ONLY s1_seq);
> >
> > May I know what the purpose of ONLY is here?
> >
> > AFAIU, for FOR ALL TABLES, ONLY was added to handle inherited tables.
> > However, sequences do not have inheritance, so I am not sure what
> > semantics ONLY is intended to provide for sequences. Could you please
> > clarify?
> It was unintentional. ONLY should not be allowed with SEQUENCE.
> This happened because, in gram.y, the EXCEPT SEQUENCE production uses
> relation_expr:
> +PublicationExceptSeqSpec:
> + relation_expr
> + {
> + $$ = makeNode(PublicationObjSpec);
> + $$->pubobjtype = PUBLICATIONOBJ_EXCEPT_SEQUENCE;
> + $$->pubrelation = makeNode(PublicationRelation);
> + $$->pubrelation->except = true;
> + $$->pubrelation->relation = $1;
> + $$->location = @1;
> + }
> + ;
> relation_expr accepts ONLY, so it inadvertently allows syntax such as
> EXCEPT (SEQUENCE ONLY s1).
> I think it would be better to use qualified_name instead. That's also
> what CREATE SEQUENCE uses, so it would be consistent with the rest of
> the grammar and would prevent ONLY from being accepted.
>
> With this change we are now throwing syntax error:
> postgres=# CREATE PUBLICATION combined_seq_pub_1
> FOR ALL SEQUENCES
> EXCEPT (SEQUENCE ONLY s1_seq);
> ERROR: syntax error at or near "ONLY"
> LINE 3: EXCEPT (SEQUENCE ONLY s1_seq);
>
> While making the change, I also found that I missed updating the
> comments in gram.y for this feature.
> I have updated the same and attached the updated v18 patches.
>

Thanks for fixing this. I found a few cosmetics issues while reviewing
the latest patches, please see if these make sense to you:

+# Check ALTER PUBLICATION ... ALL SEQUENCES (EXCEPT SEQUENCE ...)
+$node_publisher->safe_psql(

The placement of the EXCEPT clause here doesn't match the actual
supported syntax. The correct form is:

CREATE PUBLICATION combined_pub_seq FOR ALL SEQUENCES EXCEPT ( SEQUENCE ... )

--

+ * Get the list of tables and sequences for publications specified in
+ * the EXCEPT clause.

Above comment doesn't look clearer to me. Perhaps something like "for
tables and sequences specified in the publication's EXCEPT clause"
would read more clearly. What do you think?

--

+ /* Print publications where the sequence is in the EXCEPT clause */
+ if (pset.sversion >= 200000)

This doesn't read well as well, how about "Print publications whose
EXCEPT clause contains this sequence"?

--

+ * In FOR ALL TABLES/ FOR ALL SEQUENCES mode, relations are

This seems to be having uneven spacing. I would rather replace '/'
with 'or' here.

--
With Regards,
Ashutosh Sharma.