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
diff --git a/src/test/isolation/expected/drop-index-concurrently-1.out 
b/src/test/isolation/expected/drop-index-concurrently-1.out
index 97e1a6e779..1cb2250891 100644
--- a/src/test/isolation/expected/drop-index-concurrently-1.out
+++ b/src/test/isolation/expected/drop-index-concurrently-1.out
@@ -1,17 +1,17 @@
 Parsed test spec with 3 sessions
 
-starting permutation: noseq chkiso prepi preps begin explaini explains select2 
drop insert2 end2 selecti selects end
-step noseq: SET enable_seqscan = false;
+starting permutation: chkiso prepi preps begin disableseq explaini enableseq 
explains select2 drop insert2 end2 selecti selects end
 step chkiso: SELECT (setting in ('read committed','read uncommitted')) AS 
is_read_committed FROM pg_settings WHERE name = 'default_transaction_isolation';
 is_read_committed
 -----------------
 t                
 (1 row)
 
-step prepi: PREPARE getrow_idx AS SELECT * FROM test_dc WHERE data=34 ORDER BY 
id,data;
-step preps: PREPARE getrow_seq AS SELECT * FROM test_dc WHERE 
data::text=34::text ORDER BY id,data;
+step prepi: PREPARE getrow_idxscan AS SELECT * FROM test_dc WHERE data = 34 
ORDER BY id,data;
+step preps: PREPARE getrow_seqscan AS SELECT * FROM test_dc WHERE data = 34 
ORDER BY id,data;
 step begin: BEGIN;
-step explaini: EXPLAIN (COSTS OFF) EXECUTE getrow_idx;
+step disableseq: SET enable_seqscan = false;
+step explaini: EXPLAIN (COSTS OFF) EXECUTE getrow_idxscan;
 QUERY PLAN                                    
 ----------------------------------------------
 Sort                                          
@@ -20,16 +20,17 @@ Sort
         Index Cond: (data = 34)               
 (4 rows)
 
-step explains: EXPLAIN (COSTS OFF) EXECUTE getrow_seq;
-QUERY PLAN                                    
-----------------------------------------------
-Sort                                          
-  Sort Key: id, data                          
-  ->  Index Scan using test_dc_pkey on test_dc
-        Filter: ((data)::text = '34'::text)   
+step enableseq: SET enable_seqscan = true;
+step explains: EXPLAIN (COSTS OFF) EXECUTE getrow_seqscan;
+QUERY PLAN                 
+---------------------------
+Sort                       
+  Sort Key: id             
+  ->  Seq Scan on test_dc  
+        Filter: (data = 34)
 (4 rows)
 
-step select2: SELECT * FROM test_dc WHERE data=34 ORDER BY id,data;
+step select2: SELECT * FROM test_dc WHERE data = 34 ORDER BY id,data;
 id|data
 --+----
 34|  34
@@ -38,14 +39,14 @@ id|data
 step drop: DROP INDEX CONCURRENTLY test_dc_data; <waiting ...>
 step insert2: INSERT INTO test_dc(data) SELECT * FROM generate_series(1, 100);
 step end2: COMMIT;
-step selecti: EXECUTE getrow_idx;
+step selecti: EXECUTE getrow_idxscan;
  id|data
 ---+----
  34|  34
 134|  34
 (2 rows)
 
-step selects: EXECUTE getrow_seq;
+step selects: EXECUTE getrow_seqscan;
  id|data
 ---+----
  34|  34
diff --git a/src/test/isolation/expected/drop-index-concurrently-1_2.out 
b/src/test/isolation/expected/drop-index-concurrently-1_2.out
index 04612d3cac..266b0e4ada 100644
--- a/src/test/isolation/expected/drop-index-concurrently-1_2.out
+++ b/src/test/isolation/expected/drop-index-concurrently-1_2.out
@@ -1,17 +1,17 @@
 Parsed test spec with 3 sessions
 
-starting permutation: noseq chkiso prepi preps begin explaini explains select2 
drop insert2 end2 selecti selects end
-step noseq: SET enable_seqscan = false;
+starting permutation: chkiso prepi preps begin disableseq explaini enableseq 
explains select2 drop insert2 end2 selecti selects end
 step chkiso: SELECT (setting in ('read committed','read uncommitted')) AS 
is_read_committed FROM pg_settings WHERE name = 'default_transaction_isolation';
 is_read_committed
 -----------------
 f                
 (1 row)
 
-step prepi: PREPARE getrow_idx AS SELECT * FROM test_dc WHERE data=34 ORDER BY 
id,data;
-step preps: PREPARE getrow_seq AS SELECT * FROM test_dc WHERE 
data::text=34::text ORDER BY id,data;
+step prepi: PREPARE getrow_idxscan AS SELECT * FROM test_dc WHERE data = 34 
ORDER BY id,data;
+step preps: PREPARE getrow_seqscan AS SELECT * FROM test_dc WHERE data = 34 
ORDER BY id,data;
 step begin: BEGIN;
-step explaini: EXPLAIN (COSTS OFF) EXECUTE getrow_idx;
+step disableseq: SET enable_seqscan = false;
+step explaini: EXPLAIN (COSTS OFF) EXECUTE getrow_idxscan;
 QUERY PLAN                                    
 ----------------------------------------------
 Sort                                          
@@ -20,16 +20,17 @@ Sort
         Index Cond: (data = 34)               
 (4 rows)
 
-step explains: EXPLAIN (COSTS OFF) EXECUTE getrow_seq;
-QUERY PLAN                                    
-----------------------------------------------
-Sort                                          
-  Sort Key: id, data                          
-  ->  Index Scan using test_dc_pkey on test_dc
-        Filter: ((data)::text = '34'::text)   
+step enableseq: SET enable_seqscan = true;
+step explains: EXPLAIN (COSTS OFF) EXECUTE getrow_seqscan;
+QUERY PLAN                 
+---------------------------
+Sort                       
+  Sort Key: id             
+  ->  Seq Scan on test_dc  
+        Filter: (data = 34)
 (4 rows)
 
-step select2: SELECT * FROM test_dc WHERE data=34 ORDER BY id,data;
+step select2: SELECT * FROM test_dc WHERE data = 34 ORDER BY id,data;
 id|data
 --+----
 34|  34
@@ -38,13 +39,13 @@ id|data
 step drop: DROP INDEX CONCURRENTLY test_dc_data; <waiting ...>
 step insert2: INSERT INTO test_dc(data) SELECT * FROM generate_series(1, 100);
 step end2: COMMIT;
-step selecti: EXECUTE getrow_idx;
+step selecti: EXECUTE getrow_idxscan;
 id|data
 --+----
 34|  34
 (1 row)
 
-step selects: EXECUTE getrow_seq;
+step selects: EXECUTE getrow_seqscan;
 id|data
 --+----
 34|  34
diff --git a/src/test/isolation/specs/drop-index-concurrently-1.spec 
b/src/test/isolation/specs/drop-index-concurrently-1.spec
index 812b5de226..a57a02469d 100644
--- a/src/test/isolation/specs/drop-index-concurrently-1.spec
+++ b/src/test/isolation/specs/drop-index-concurrently-1.spec
@@ -3,7 +3,8 @@
 # This test shows that the concurrent write behaviour works correctly
 # with the expected output being 2 rows at the READ COMMITTED and READ
 # UNCOMMITTED transaction isolation levels, and 1 row at the other
-# transaction isolation levels.
+# transaction isolation levels.  We ensure this is the case by checking
+# the returned rows in an index scan plan and a seq scan plan.
 #
 setup
 {
@@ -18,24 +19,25 @@ teardown
 }
 
 session s1
-step noseq { SET enable_seqscan = false; }
 step chkiso { SELECT (setting in ('read committed','read uncommitted')) AS 
is_read_committed FROM pg_settings WHERE name = 
'default_transaction_isolation'; }
-step prepi { PREPARE getrow_idx AS SELECT * FROM test_dc WHERE data=34 ORDER 
BY id,data; }
-step preps { PREPARE getrow_seq AS SELECT * FROM test_dc WHERE 
data::text=34::text ORDER BY id,data; }
+step prepi { PREPARE getrow_idxscan AS SELECT * FROM test_dc WHERE data = 34 
ORDER BY id,data; }
+step preps { PREPARE getrow_seqscan AS SELECT * FROM test_dc WHERE data = 34 
ORDER BY id,data; }
 step begin { BEGIN; }
-step explaini { EXPLAIN (COSTS OFF) EXECUTE getrow_idx; }
-step explains { EXPLAIN (COSTS OFF) EXECUTE getrow_seq; }
-step selecti { EXECUTE getrow_idx; }
-step selects { EXECUTE getrow_seq; }
+step disableseq { SET enable_seqscan = false; }
+step explaini { EXPLAIN (COSTS OFF) EXECUTE getrow_idxscan; }
+step enableseq { SET enable_seqscan = true; }
+step explains { EXPLAIN (COSTS OFF) EXECUTE getrow_seqscan; }
+step selecti { EXECUTE getrow_idxscan; }
+step selects { EXECUTE getrow_seqscan; }
 step end { COMMIT; }
 
 session s2
 setup { BEGIN; }
-step select2 { SELECT * FROM test_dc WHERE data=34 ORDER BY id,data; }
+step select2 { SELECT * FROM test_dc WHERE data = 34 ORDER BY id,data; }
 step insert2 { INSERT INTO test_dc(data) SELECT * FROM generate_series(1, 
100); }
 step end2 { COMMIT; }
 
 session s3
 step drop { DROP INDEX CONCURRENTLY test_dc_data; }
 
-permutation noseq chkiso prepi preps begin explaini explains select2 drop 
insert2 end2 selecti selects end
+permutation chkiso prepi preps begin disableseq explaini enableseq explains 
select2 drop insert2 end2 selecti selects end

Reply via email to