From 807a40c551dd30c8dd5a0b3bd82f5bbb1e7fd285 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 18 Sep 2012 12:20:34 -0400
Subject: [PATCH] Fix planning of btree index scans using ScalarArrayOpExpr
quals.
In commit 9e8da0f75731aaa7605cf4656c21ea09e84d2eb1, I improved btree
to handle ScalarArrayOpExpr quals natively, so that constructs like
"indexedcol IN (list)" could be supported by index-only scans. Using
such a qual results in multiple scans of the index, under-the-hood.
I went to some lengths to ensure that this still produces rows in index
order ... but I failed to recognize that if a higher-order index column
is lacking an equality constraint, rescans can produce out-of-order
data from that column. Tweak the planner to not expect sorted output
in that case. Per trouble report from Robert McGehee.
---
src/backend/optimizer/path/indxpath.c | 15 +++++++-
src/test/regress/expected/create_index.out | 45 ++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 24 ++++++++++++
3 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 53bf15aea23..444ec2a40e4 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -787,6 +787,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
List *index_pathkeys;
List *useful_pathkeys;
bool found_clause;
+ bool found_lower_saop_clause;
bool pathkeys_possibly_useful;
bool index_is_ordered;
bool index_only_scan;
@@ -824,6 +825,13 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
* if saop_control is SAOP_REQUIRE, it has to be a ScalarArrayOpExpr
* clause.
*
+ * found_lower_saop_clause is set true if there's a ScalarArrayOpExpr
+ * index clause for a non-first index column. This prevents us from
+ * assuming that the scan result is ordered. (Actually, the result is
+ * still ordered if there are equality constraints for all earlier
+ * columns, but it seems too expensive and non-modular for this code to be
+ * aware of that refinement.)
+ *
* We also build a Relids set showing which outer rels are required by the
* selected clauses. Any lateral_relids are included in that, but not
* otherwise accounted for.
@@ -831,6 +839,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
index_clauses = NIL;
clause_columns = NIL;
found_clause = false;
+ found_lower_saop_clause = false;
outer_relids = bms_copy(rel->lateral_relids);
for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
{
@@ -846,6 +855,8 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
if (saop_control == SAOP_PER_AM && !index->amsearcharray)
continue;
found_clause = true;
+ if (indexcol > 0)
+ found_lower_saop_clause = true;
}
else
{
@@ -882,9 +893,11 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
/*
* 2. Compute pathkeys describing index's ordering, if any, then see how
* many of them are actually useful for this query. This is not relevant
- * if we are only trying to build bitmap indexscans.
+ * if we are only trying to build bitmap indexscans, nor if we have to
+ * assume the scan is unordered.
*/
pathkeys_possibly_useful = (scantype != ST_BITMAPSCAN &&
+ !found_lower_saop_clause &&
has_useful_pathkeys(root, rel));
index_is_ordered = (index->sortopfamily != NULL);
if (index_is_ordered && pathkeys_possibly_useful)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 999e38a6798..2ae991eebe2 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2676,3 +2676,48 @@ SELECT count(*) FROM dupindexcols
97
(1 row)
+--
+-- Check ordering of =ANY indexqual results (bug in 9.2.0)
+--
+vacuum analyze tenk1; -- ensure we get consistent plans here
+explain (costs off)
+SELECT unique1 FROM tenk1
+WHERE unique1 IN (1,42,7)
+ORDER BY unique1;
+ QUERY PLAN
+-------------------------------------------------------
+ Index Only Scan using tenk1_unique1 on tenk1
+ Index Cond: (unique1 = ANY ('{1,42,7}'::integer[]))
+(2 rows)
+
+SELECT unique1 FROM tenk1
+WHERE unique1 IN (1,42,7)
+ORDER BY unique1;
+ unique1
+---------
+ 1
+ 7
+ 42
+(3 rows)
+
+explain (costs off)
+SELECT thousand, tenthous FROM tenk1
+WHERE thousand < 2 AND tenthous IN (1001,3000)
+ORDER BY thousand;
+ QUERY PLAN
+--------------------------------------------------------------------------------------
+ Sort
+ Sort Key: thousand
+ -> Index Only Scan using tenk1_thous_tenthous on tenk1
+ Index Cond: ((thousand < 2) AND (tenthous = ANY ('{1001,3000}'::integer[])))
+(4 rows)
+
+SELECT thousand, tenthous FROM tenk1
+WHERE thousand < 2 AND tenthous IN (1001,3000)
+ORDER BY thousand;
+ thousand | tenthous
+----------+----------
+ 0 | 3000
+ 1 | 1001
+(2 rows)
+
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index f8edb7ee766..914e7a57a47 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -888,3 +888,27 @@ EXPLAIN (COSTS OFF)
WHERE f1 > 'WA' and id < 1000 and f1 ~<~ 'YX';
SELECT count(*) FROM dupindexcols
WHERE f1 > 'WA' and id < 1000 and f1 ~<~ 'YX';
+
+--
+-- Check ordering of =ANY indexqual results (bug in 9.2.0)
+--
+
+vacuum analyze tenk1; -- ensure we get consistent plans here
+
+explain (costs off)
+SELECT unique1 FROM tenk1
+WHERE unique1 IN (1,42,7)
+ORDER BY unique1;
+
+SELECT unique1 FROM tenk1
+WHERE unique1 IN (1,42,7)
+ORDER BY unique1;
+
+explain (costs off)
+SELECT thousand, tenthous FROM tenk1
+WHERE thousand < 2 AND tenthous IN (1001,3000)
+ORDER BY thousand;
+
+SELECT thousand, tenthous FROM tenk1
+WHERE thousand < 2 AND tenthous IN (1001,3000)
+ORDER BY thousand;
--
GitLab