On Monday, February 8, 2021 1:44 PM osumi.takami...@fujitsu.com <osumi.takami...@fujitsu.com> > On Mon, Feb 8, 2021 12:40 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > > On Mon, Feb 8, 2021 at 8:06 AM Peter Smith <smithpb2...@gmail.com> > > wrote: > > > > > > On Sat, Feb 6, 2021 at 6:30 PM osumi.takami...@fujitsu.com > > > <osumi.takami...@fujitsu.com> wrote: > > > > > > > > > I have another idea for a test case: What if we write a test > > > > > such that it fails PK violation on copy and then drop the > > > > > subscription. > > > > > Then check there shouldn't be any dangling slot on the publisher? > > > > > This is similar to a test in subscription/t/004_sync.pl, we can > > > > > use some of that framework but have a separate test for this. > > > > I've added this PK violation test to the attached tests. > > > > The patch works with v28 and made no failure during regression tests. > > > > > > > > > > I checked this patch. It applied cleanly on top of V28, and all > > > tests passed > > OK. > > > > > > Here are two feedback comments. > > > > > > 1. For the regression test there is 2 x SQL and 1 x function test. I > > > thought to cover all the combinations there should be another > > > function test. e.g. > > > Tests ALTER … REFRESH > > > Tests ALTER …. (refresh = true) > > > Tests ALTER … (refresh = true) in a function Tests ALTER … REFRESH > > > in a function <== this combination is not being testing ?? > > > > > > > I am not sure whether there is much value in adding more to this set > > of negative test cases unless it really covers a different code path > > which I think won't happen if we add more tests here. > Yeah, I agree. Accordingly, I didn't fix that part. > > > On Mon, Feb 8, 2021 11:36 AM Peter Smith <smithpb2...@gmail.com> > wrote: > > 2. For the 004 test case I know the test is needing some PK constraint > > violation # Check if DROP SUBSCRIPTION cleans up slots on the > > publisher side # when the subscriber is stuck on data copy for > > constraint > > > > But it is not clear to me what was the exact cause of that PK > > violation. I think you must be relying on data that is leftover from > > some previous test case but I am not sure which one. Can you make the > > comment more detailed to say > > *how* the PK violation is happening - e.g something to say which rows, > > in which table, and inserted by who? > I added some comments to clarify how the PK violation happens. > Please have a look. Sorry, I had a one typo in the tests of subscription.sql in v2. I used 'foo' for the first test of "ALTER SUBSCRIPTION mytest SET PUBLICATION foo WITH (refresh = true) in v02", but I should have used 'mypub' to make this test clearly independent from other previous tests. Attached the fixed version.
Best Regards,
Takamichi Osumi
refresh_and_pk_violation_testsets_v03.patch
Description: refresh_and_pk_violation_testsets_v03.patch
