On Tuesday, September 26, 2023 4:40 PM Amit Kapila <amit.kapil...@gmail.com>
wrote:
>
> On Tue, Sep 19, 2023 at 12:48 PM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com>
> wrote:
> >
> > - static bool publish_no_origin;
> >
> > This flag is also local to pgoutput instance, and we didn't reset the
> > flag in output shutdown callback, so if we consume changes from
> > different slots, then the second call would reuse the flag value that
> > is set in the first call which is unexpected. To completely avoid this
> > issue, we think we'd better move this flag to output plugin private data
> structure.
> >
> > Example:
> > SELECT data FROM pg_logical_slot_peek_binary_changes('isolation_slot_1',
> NULL, NULL, 'proto_version', '1', 'publication_names', 'pub', 'origin',
> 'none'); ---
> Set origin in this call.
> > SELECT data FROM pg_logical_slot_peek_binary_changes('isolation_slot_2',
> NULL, NULL, 'proto_version', '1', 'publication_names', 'pub'); -- Didn't set
> origin, but will reuse the origin flag in the first call.
> >
>
> char *origin;
> + bool publish_no_origin;
> } PGOutputData;
>
> Do we really need a new parameter in above structure? Can't we just use the
> existing origin in the same structure? Please remember if this needs to be
> backpatched then it may not be good idea to add new parameter in the
> structure but apart from that having two members to represent similar
> information doesn't seem advisable to me. I feel for backbranch we can just
> use
> PGOutputData->origin for comparison and for HEAD, we can remove origin
> and just use a boolean to avoid any extra cost for comparisions for each
> change.OK, I agree to remove the origin string on HEAD and we can add that back when we support other origin value. I also modified to use the string for comparison as suggested for back-branch. I will also test it locally to confirm it doesn't affect the perf. > > Can we add a test case to cover this case? Added one in replorigin.sql. Attach the patch set for publish_no_origin and in_streaming including the patch(v2-PG16-0001) for back-branches. Since the patch for hash table need more thoughts, I didn't post it this time. Best Regards, Hou zj
v2-0001-Maintain-publish_no_origin-in-output-plugin-priv.patch
Description: v2-0001-Maintain-publish_no_origin-in-output-plugin-priv.patch
v2-PG16-0001-Maintain-publish_no_origin-in-output-plugin-priva.patch
Description: v2-PG16-0001-Maintain-publish_no_origin-in-output-plugin-priva.patch
v2-0002-Move-in_streaming-to-output-private-data.patch
Description: v2-0002-Move-in_streaming-to-output-private-data.patch
