On Fri, 19 Dec 2025 at 10:05, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Vignesh.
>
> I had a quick look at this patch.
>
> ======
>
> - * Instead, we invalidate only the relsyncache.
> + * Instead, invalidate the relation sync cache for publications that
> + * include tables. Invalidation is not required for sequences only
> + * publications.
> */
> - InvalidatePubRelSyncCache(pub->oid, pub->puballtables);
> + if (!pub->puballsequences || pub->puballtables)
> + InvalidatePubRelSyncCache(pub->oid, pub->puballtables);
>
> I felt all these "sequence only" conditions are becoming difficult to read.
>
> It wonder if it would be better to introduce some function like:
>
> bool
> PubHasSequencesOnly(pub)
> {
> return pub->puballsequences && !pub->puballtables;
> }
I added a macro following a similar pattern. However, since the
function calls use different data types, I modified it to pass
individual members instead of the structure.
> IIUC the current patch code only works because publication syntax like
> below is not yet supported:
>
> CREATE PUBLICATION pub1 FOR ALL SEQUENCES, TABLE t1;
>
> But when that syntax does become possible, then all these conditions
> will be broken.
>
> Should we prepare for that eventuality by introducing some function
> right now, so as to contain all the future broken code?
I believe no specific handling is required. These flows will be
naturally addressed as part of the feature implementation.
The attached patch has the changes for the comments.
Regards,
Vignesh
