Re: Proposal: Conflict log history table for Logical Replication

Поиск
Список
Период
Сортировка
От Dilip Kumar
Тема Re: Proposal: Conflict log history table for Logical Replication
Дата
Msg-id CAFiTN-sL701K8=qx8Ffyg0Jgr1x=HrRuGP5d2W3_D-7ZeYKahQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Proposal: Conflict log history table for Logical Replication  (Amit Kapila <amit.kapila16@gmail.com>)
Ответы Re: Proposal: Conflict log history table for Logical Replication
Список pgsql-hackers
On Mon, Dec 15, 2025 at 3:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sun, Dec 14, 2025 at 9:16 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > Here is the patch which implements the dependency and fixes other
> > comments from Shveta.
> >
>
> +/*
> + * Check if the specified relation is used as a conflict log table by any
> + * subscription.
> + */
> +bool
> +IsConflictLogTable(Oid relid)
> +{
> + Relation rel;
> + TableScanDesc scan;
> + HeapTuple tup;
> + bool is_clt = false;
> +
> + rel = table_open(SubscriptionRelationId, AccessShareLock);
> + scan = table_beginscan_catalog(rel, 0, NULL);
> +
> + while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection)))
>
> This function has been used at multiple places in the patch, though
> not in any performance-critical paths, but still, it seems like the
> impact can be noticeable for a large number of subscriptions. Also, I
> am not sure it is a good design to scan the entire system table to
> find whether some other relation is publishable or not. I see below
> kinds of usages for it:
>
> + /* Subscription conflict log tables are not published */
> + result = is_publishable_class(relid, (Form_pg_class) GETSTRUCT(tuple)) &&
> + !IsConflictLogTable(relid);
>
> In this regard, I see a comment atop is_publishable_class which
> suggests as follows:
>
> The best
>  * long-term solution may be to add a "relispublishable" bool to pg_class,
>  * and depend on that instead of OID checks.
>  */
> static bool
> is_publishable_class(Oid relid, Form_pg_class reltuple)
>
> I feel that is a good idea for reasons mentioned atop
> is_publishable_class and for the conflict table. What do you think?

On quick thought, this seems like a good idea and may simplify a
couple of places.  And might be good for future extension as we can
mark publishable at individual relation instead of targeting broad
categories like IsCatalogRelationOid() or checking individual items by
its Oid.  IMHO this can be done as an individual patch in a separate
thread, or as a base patch.

--
Regards,
Dilip Kumar
Google



В списке pgsql-hackers по дате отправления: