public inbox for [email protected]  

help / color / mirror / Atom feed

From: Shlok Kyal <[email protected]>
To: Shubham Khanna <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Rajendra Kumar Dangwal <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: Pgoutput not capturing the generated columns
Date: Mon, 17 Jun 2024 11:50:33 +0530
Message-ID: <CANhcyEWm+EUuWViQH9mTuzLAtvTrDCr5vLPoeeVMRK=aUzbRdA@mail.gmail.com> (raw)
In-Reply-To: <CAHv8RjJn6EiyAitJbbvkvVV2d45fV3Wjr2VmWFugm3RsbaU+Rg@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CAHv8RjKEBtFfUbmdHPYV1K76bb8PEm6R=iLZko4Dxy2Oh-_uUw@mail.gmail.com>
	<CAHut+PsuJfcaeg6zst=6PE5uyJv_UxVRHU3ck7W2aHb1uQYKng@mail.gmail.com>
	<CAHv8RjJcOsk=y+vJ3y+vXhzR9ZUzUEURvS_90hQW3MNfJ5di7A@mail.gmail.com>
	<CALDaNm3-qiNuf4Wy2E1HtG=SLBJEFGnh010Fqc8eO=Lyg5+pfQ@mail.gmail.com>
	<CAHv8RjJn6EiyAitJbbvkvVV2d45fV3Wjr2VmWFugm3RsbaU+Rg@mail.gmail.com>

On Fri, 14 Jun 2024 at 15:52, Shubham Khanna
<[email protected]> wrote:
>
> The attached Patch contains the suggested changes.
>

Hi Shubham, thanks for providing a patch.
I have some comments for v6-0001.

1. create_subscription.sgml
There is repetition of the same line.

+         <para>
+          Specifies whether the generated columns present in the tables
+          associated with the subscription should be replicated. If the
+          subscriber-side column is also a generated column then this option
+          has no effect; the replicated data will be ignored and the subscriber
+          column will be filled as normal with the subscriber-side computed or
+          default data.
+          <literal>false</literal>.
+         </para>
+
+         <para>
+          This parameter can only be set true if
<literal>copy_data</literal> is
+          set to <literal>false</literal>. If the subscriber-side
column is also a
+          generated column then this option has no effect; the
replicated data will
+          be ignored and the subscriber column will be filled as
normal with the
+          subscriber-side computed or default data.
+         </para>

==============================
2. subscriptioncmds.c

2a. The macro name should be in uppercase. We can use a short name
like 'SUBOPT_INCLUDE_GEN_COL'. Thought?
+#define SUBOPT_include_generated_columns 0x00010000

2b.Update macro name accordingly
+ if (IsSet(supported_opts, SUBOPT_include_generated_columns))
+ opts->include_generated_columns = false;

2c. Update macro name accordingly
+ else if (IsSet(supported_opts, SUBOPT_include_generated_columns) &&
+ strcmp(defel->defname, "include_generated_columns") == 0)
+ {
+ if (IsSet(opts->specified_opts, SUBOPT_include_generated_columns))
+ errorConflictingDefElem(defel, pstate);
+
+ opts->specified_opts |= SUBOPT_include_generated_columns;
+ opts->include_generated_columns = defGetBoolean(defel);
+ }

2d. Update macro name accordingly
+   SUBOPT_RUN_AS_OWNER | SUBOPT_FAILOVER | SUBOPT_ORIGIN |
+   SUBOPT_include_generated_columns);


==============================

3. decoding_into_rel.out

3a. In comment, I think it should be "When 'include-generated-columns'
= '1' the generated column 'b' values will be replicated"
+-- When 'include-generated-columns' = '1' the generated column 'b'
values will not be replicated
+INSERT INTO gencoltable (a) VALUES (1), (2), (3);
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
NULL, 'include-xids', '0', 'skip-empty-xacts', '1',
'include-generated-columns', '1');
+                            data
+-------------------------------------------------------------
+ BEGIN
+ table public.gencoltable: INSERT: a[integer]:1 b[integer]:2
+ table public.gencoltable: INSERT: a[integer]:2 b[integer]:4
+ table public.gencoltable: INSERT: a[integer]:3 b[integer]:6
+ COMMIT

3b. In comment, I think it should be "When 'include-generated-columns'
= '1' the generated column 'b' values will not be replicated"
+-- When 'include-generated-columns' = '0' the generated column 'b'
values will be replicated
+SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
NULL, 'include-xids', '0', 'skip-empty-xacts', '1',
'include-generated-columns', '0');
+                      data
+------------------------------------------------
+ BEGIN
+ table public.gencoltable: INSERT: a[integer]:4
+ table public.gencoltable: INSERT: a[integer]:5
+ table public.gencoltable: INSERT: a[integer]:6
+ COMMIT
+(5 rows)

=========================

4. Here names for both the tests are the same. I think we should use
different names.

+$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM tab2");
+is( $result, qq(4|8
+5|10), 'generated columns replicated to non-generated column on subscriber');
+
+$node_publisher->safe_psql('postgres', "INSERT INTO tab3 VALUES (4), (5)");
+
+$node_publisher->wait_for_catchup('sub3');
+
+$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM tab3");
+is( $result, qq(4|24
+5|25), 'generated columns replicated to non-generated column on subscriber');

Thanks and Regards,
Shlok Kyal

view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Pgoutput not capturing the generated columns
  In-Reply-To: <CANhcyEWm+EUuWViQH9mTuzLAtvTrDCr5vLPoeeVMRK=aUzbRdA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox