The drop-index-concurrently-1 isolation test no longer tests what it was meant to - Mailing list pgsql-hackers

From David Rowley
Subject The drop-index-concurrently-1 isolation test no longer tests what it was meant to
Date
Msg-id CAApHDvrbDhObhLV+=U_K_-t+2Av2av1aL9d+2j_3AO-XndaviA@mail.gmail.com
Whole thread Raw
Responses Re: The drop-index-concurrently-1 isolation test no longer tests what it was meant to  (David Rowley <dgrowleyml@gmail.com>)
List pgsql-hackers
I'm in the middle of working on making some adjustments to the costs
of Incremental Sorts and I see the patch I wrote changes the plan in
the drop-index-concurrently-1 isolation test.

The particular plan changed currently expects:

-----------------------------------------------
Sort
  Sort Key: id, data
  ->  Index Scan using test_dc_pkey on test_dc
        Filter: ((data)::text = '34'::text)

It seems fairly clear from reading the test spec that this plan is
really meant to be a seq scan plan and the change made in [1] adjusted
that without any regard for that.

That seems to have come around because of how the path generation of
incremental sorts work.  The current incremental sort path generation
will put a Sort path atop of the cheapest input path, even if that
cheapest input path has presorted keys. The test_dc_pkey index
provides presorted input for the required sort order.  Prior to
incremental sort, we did not consider paths which only provided
presorted input to be useful paths, hence we used to get a seq scan
plan.

I propose the attached which gets rid of the not-so-great casting
method that was originally added to this test to try and force the seq
scan.  It seems a little dangerous to put in hacks like that to force
a particular plan when the resulting plan ends up penalized with a
(1.0e10) disable_cost.  The planner is just not going to be stable
when the plan includes such a large penalty. To force the planner,
I've added another test step to do set enable_seqscan to true and
adjusted the permutations to run that just before preparing the seq
scan query.

I also tried to make it more clear that we want to be running the
query twice, once with an index scan and again with a seq scan. I'm
hoping the changes to the prepared query names and the extra comments
will help reduce the chances of this getting broken again in the
future.

David

[1]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blobdiff;f=src/test/isolation/expected/drop-index-concurrently-1.out;h=8e6adb66bb1479b8d7db2fcf5f70b89acd3af577;hp=75dff56bc46d40aa8eb012543044b7c10d516b7e;hb=d2d8a229bc5;hpb=3c8553547b1493c4afdb80393f4a47dbfa019a79

Attachment

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
Next
From: Zheng Li
Date:
Subject: Re: Support logical replication of DDLs