From 0777f7a2e8e0a51f0f60cfe164d538bb459bf9f2 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 15 Jan 2017 14:09:35 -0500
Subject: [PATCH] Fix matching of boolean index columns to sort ordering.
Normally, if we have a WHERE clause like "indexcol = constant",
the planner will figure out that that index column can be ignored
when determining whether the index has a desired sort ordering.
But this failed to work for boolean index columns, because a
condition like "boolcol = true" is canonicalized to just "boolcol"
which does not give rise to an EquivalenceClass. Add a check to
allow the same type of deduction to be made in this case too.
Per a complaint from Dima Pavlov. Arguably this is a bug, but given the
limited impact and the small number of complaints so far, I won't risk
destabilizing plans in stable branches by back-patching.
Patch by me, reviewed by Michael Paquier
Discussion: https://postgr.es/m/1788.1481605684@sss.pgh.pa.us
---
src/backend/optimizer/path/indxpath.c | 46 ++++++++++++++++++++++
src/backend/optimizer/path/pathkeys.c | 35 ++++++++++------
src/include/optimizer/paths.h | 2 +
src/test/regress/expected/create_index.out | 42 ++++++++++++++++++++
src/test/regress/sql/create_index.sql | 15 +++++++
5 files changed, 129 insertions(+), 11 deletions(-)
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 7b43c4acb5d..0a5c05033a0 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -3025,6 +3025,52 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
return false;
}
+/*
+ * indexcol_is_bool_constant_for_query
+ *
+ * If an index column is constrained to have a constant value by the query's
+ * WHERE conditions, then it's irrelevant for sort-order considerations.
+ * Usually that means we have a restriction clause WHERE indexcol = constant,
+ * which gets turned into an EquivalenceClass containing a constant, which
+ * is recognized as redundant by build_index_pathkeys(). But if the index
+ * column is a boolean variable (or expression), then we are not going to
+ * see WHERE indexcol = constant, because expression preprocessing will have
+ * simplified that to "WHERE indexcol" or "WHERE NOT indexcol". So we are not
+ * going to have a matching EquivalenceClass (unless the query also contains
+ * "ORDER BY indexcol"). To allow such cases to work the same as they would
+ * for non-boolean values, this function is provided to detect whether the
+ * specified index column matches a boolean restriction clause.
+ */
+bool
+indexcol_is_bool_constant_for_query(IndexOptInfo *index, int indexcol)
+{
+ ListCell *lc;
+
+ /* If the index isn't boolean, we can't possibly get a match */
+ if (!IsBooleanOpfamily(index->opfamily[indexcol]))
+ return false;
+
+ /* Check each restriction clause for the index's rel */
+ foreach(lc, index->rel->baserestrictinfo)
+ {
+ RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
+
+ /*
+ * As in match_clause_to_indexcol, never match pseudoconstants to
+ * indexes. (It might be semantically okay to do so here, but the
+ * odds of getting a match are negligible, so don't waste the cycles.)
+ */
+ if (rinfo->pseudoconstant)
+ continue;
+
+ /* See if we can match the clause's expression to the index column */
+ if (match_boolean_index_clause((Node *) rinfo->clause, indexcol, index))
+ return true;
+ }
+
+ return false;
+}
+
/****************************************************************************
* ---- ROUTINES TO CHECK OPERANDS ----
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 012cb62f89a..1065b31ad10 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -480,17 +480,30 @@ build_index_pathkeys(PlannerInfo *root,
index->rel->relids,
false);
- /*
- * If the sort key isn't already present in any EquivalenceClass, then
- * it's not an interesting sort order for this query. So we can stop
- * now --- lower-order sort keys aren't useful either.
- */
- if (!cpathkey)
- break;
-
- /* Add to list unless redundant */
- if (!pathkey_is_redundant(cpathkey, retval))
- retval = lappend(retval, cpathkey);
+ if (cpathkey)
+ {
+ /*
+ * We found the sort key in an EquivalenceClass, so it's relevant
+ * for this query. Add it to list, unless it's redundant.
+ */
+ if (!pathkey_is_redundant(cpathkey, retval))
+ retval = lappend(retval, cpathkey);
+ }
+ else
+ {
+ /*
+ * Boolean index keys might be redundant even if they do not
+ * appear in an EquivalenceClass, because of our special treatment
+ * of boolean equality conditions --- see the comment for
+ * indexcol_is_bool_constant_for_query(). If that applies, we can
+ * continue to examine lower-order index columns. Otherwise, the
+ * sort key is not an interesting sort order for this query, so we
+ * should stop considering index columns; any lower-order sort
+ * keys won't be useful either.
+ */
+ if (!indexcol_is_bool_constant_for_query(index, i))
+ break;
+ }
i++;
}
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 480f25f942b..81a9be7c674 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -66,6 +66,8 @@ extern void create_index_paths(PlannerInfo *root, RelOptInfo *rel);
extern bool relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
List *restrictlist,
List *exprlist, List *oprlist);
+extern bool indexcol_is_bool_constant_for_query(IndexOptInfo *index,
+ int indexcol);
extern bool match_index_to_operand(Node *operand, int indexcol,
IndexOptInfo *index);
extern void expand_indexqual_conditions(IndexOptInfo *index,
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index e663f9a3d08..c6dfb26dd55 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2952,6 +2952,48 @@ explain (costs off)
Index Cond: ((thousand = 1) AND (tenthous = 1001))
(2 rows)
+--
+-- Check matching of boolean index columns to WHERE conditions and sort keys
+--
+create temp table boolindex (b bool, i int, unique(b, i), junk float);
+explain (costs off)
+ select * from boolindex order by b, i limit 10;
+ QUERY PLAN
+-------------------------------------------------------
+ Limit
+ -> Index Scan using boolindex_b_i_key on boolindex
+(2 rows)
+
+explain (costs off)
+ select * from boolindex where b order by i limit 10;
+ QUERY PLAN
+-------------------------------------------------------
+ Limit
+ -> Index Scan using boolindex_b_i_key on boolindex
+ Index Cond: (b = true)
+ Filter: b
+(4 rows)
+
+explain (costs off)
+ select * from boolindex where b = true order by i desc limit 10;
+ QUERY PLAN
+----------------------------------------------------------------
+ Limit
+ -> Index Scan Backward using boolindex_b_i_key on boolindex
+ Index Cond: (b = true)
+ Filter: b
+(4 rows)
+
+explain (costs off)
+ select * from boolindex where not b order by i limit 10;
+ QUERY PLAN
+-------------------------------------------------------
+ Limit
+ -> Index Scan using boolindex_b_i_key on boolindex
+ Index Cond: (b = false)
+ Filter: (NOT b)
+(4 rows)
+
--
-- REINDEX (VERBOSE)
--
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 71f4f54cada..822c34af237 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1010,6 +1010,21 @@ RESET enable_indexscan;
explain (costs off)
select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null));
+--
+-- Check matching of boolean index columns to WHERE conditions and sort keys
+--
+
+create temp table boolindex (b bool, i int, unique(b, i), junk float);
+
+explain (costs off)
+ select * from boolindex order by b, i limit 10;
+explain (costs off)
+ select * from boolindex where b order by i limit 10;
+explain (costs off)
+ select * from boolindex where b = true order by i desc limit 10;
+explain (costs off)
+ select * from boolindex where not b order by i limit 10;
+
--
-- REINDEX (VERBOSE)
--
--
GitLab