| From: | "Haoyan Wang" <wanghaoyan20(at)163(dot)com> |
|---|---|
| To: | "Masahiko Sawada" <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, "Amit Kapila" <amit(dot)kapila16(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "Jan Wieck" <jan(at)wi3ck(dot)info>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Initial COPY of Logical Replication is too slow |
| Date: | 2026-03-31 06:34:47 |
| Message-ID: | 4e491308.5ab6.19d429a167a.Coremail.wanghaoyan20@163.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
At 2026-03-31 14:15:18, "王好燕 " <wanghaoyan20(at)163(dot)com> wrote:
At 2026-03-31 12:08:52, "Masahiko Sawada" <sawada(dot)mshk(at)gmail(dot)com> wrote:
>On Mon, Mar 30, 2026 at 12:16 AM Zhijie Hou (Fujitsu)
><houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>>
>> On Friday, March 27, 2026 2:20 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>> > I've attached the updated patch. I believe I've addressed all comments I got
>> > so far. In addition to that, I've refactored
>> > is_table_publishable_in_publication() and added more regression tests.
>>
>> Thanks for updating the patch.
>>
>> The latest patch looks mostly good to me. However, I noticed one issue: the
>> function returns table information even for unlogged or temporary tables. I
>> think we should return NULL for those cases instead.
>
>Indeed. Good catch!
>
>>
>> BTW, I think we could use is_publishable_class() as a reference to check once
>> whether all unpublishable table types are properly ignored in this function.
>>
>
>+1. I've added is_publishable_class() check.
>
>I've attached the updated patch.
>
>Regards,
>
>--
>Masahiko Sawada
>Amazon Web Services: https://aws.amazon.com
Hi, thanks for the patch. I just reviewed v6, here comes some comments.
+/*
+ * Similar to is_publishable_calss() but checks whether the given OID
+ * is a publishable "table" or not.
+ */
+static bool
+is_publishable_table(Oid tableoid)
+{
+ HeapTuple tuple;
+ Form_pg_class relform;
+
+ tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(tableoid));
+ if (!HeapTupleIsValid(tuple))
+ return false;
+
+ relform = (Form_pg_class) GETSTRUCT(tuple);
+
+ /*
+ * Sequences are publishable according to is_publishable_class() so
+ * explicitly exclude here.
+ */
+ if (relform->relkind != RELKIND_SEQUENCE &&
+ is_publishable_class(tableoid, relform))
+ {
+ ReleaseSysCache(tuple);
+ return true;
+ }
+
+ ReleaseSysCache(tuple);
+ return true;
+}
I think the last return should return false.
+ * Similar to is_publishable_calss() but checks whether the given OID
“calss" should be “class”.
One more nit comment on the commit message:
"The existing a VARIADIC array form of pg_get_publication_tables() is"
In this line, “a” seems not needed.
Regards,
Haoyan Wang
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexander Lakhin | 2026-03-31 07:00:00 | Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE |
| Previous Message | Andreas Karlsson | 2026-03-31 06:30:25 | Re: Improve pgindent's formatting named fields in struct literals and varidic functions |