From: | Shubham Khanna <khannashubham1197(at)gmail(dot)com> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | vignesh C <vignesh21(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Rajendra Kumar Dangwal <dangwalrajendra888(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "euler(at)eulerto(dot)com" <euler(at)eulerto(dot)com> |
Subject: | Re: Pgoutput not capturing the generated columns |
Date: | 2024-08-22 04:58:16 |
Message-ID: | CAHv8RjKMN7B-Avg50Z5w3V=tP25_do8-zTi=QCEbqbtwVpfTkw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Aug 19, 2024 at 12:40 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Shubham, here are my review comments for the TAP tests patch v27-0002
>
> ======
> Commit message
>
> Tap tests for 'include-generated-columns'
>
> ~
>
> But, it's more than that-- these are the TAP tests for all
> combinations of replication related to generated columns. i.e. both
> with and without 'include_generated_columns' option enabled.
>
> ======
> src/test/subscription/t/011_generated.pl
>
> I was mistaken, thinking that the v27-0002 had already been refactored
> according to Vignesh's last review but it is not done yet, so I am not
> going to post detailed review comments until the restructuring is
> completed.
>
> ~
>
> OTOH, there are some problems I felt have crept into v26-0001 (TAP
> test is same as v27-0002), so maybe try to also take care of them (see
> below) in v28-0002.
>
> In no particular order:
>
> * I felt it is almost useless now to have the "combo" (
> "regress_pub_combo") publication. It used to have many tables when
> you first created it but with every version posted it is publishing
> less and less so now there are only 2 tables in it. Better to have a
> specific publication for each table now and forget about "combos"
>
> * The "TEST tab_gen_to_gen initial sync" seems to be not even checking
> the table data. Why not? e.g. Even if you expect no data, you should
> test for it.
>
> * The "TEST tab_gen_to_gen replication" seems to be not even checking
> the table data. Why not?
>
> * Multiple XXX comments like "... it needs more study to determine if
> the above result was actually correct, or a PG17 bug..." should be
> removed. AFAIK we should well understand the expected results for all
> combinations by now.
>
> * The "TEST tab_order replication" is now getting an error saying
> <missing replicated column: "c">, Now, that may now be the correct
> error for this situation, but in that case, then I think the test is
> not longer testing what it was intended to test (i.e. that column
> order does not matter....) Probably the table definition needs
> adjusting to make sure we are testing whenwe want to test, and not
> just making some random scenario "PASS".
>
> * The test "# TEST tab_alter" expected empty result also seems
> unhelpful. It might be related to the previous bullet.
I have addressed all the comments in the v-28-0002 Patch. Please refer
to the updated v28-0002 Patch here in [1]. See [1] for the changes
added.
Thanks and Regards,
Shubham Khanna.
From | Date | Subject | |
---|---|---|---|
Next Message | jian he | 2024-08-22 05:19:00 | Re: Emitting JSON to file using COPY TO |
Previous Message | Shubham Khanna | 2024-08-22 04:56:24 | Re: Pgoutput not capturing the generated columns |