Hi,
It took me a while but I finally got back to working on this patch. I
reread the summary of principles Peter shared in August, and I do agree
with all the main points - the overall qual hierarchy and the various
examples and counter-examples.
I'll respond to a couple things in this sub-thread, and then I plan to
post an updated version of the patch with a discussion of a couple
problems I still haven't managed to solve (not necessarily new ones).
On 8/19/23 00:19, Peter Geoghegan wrote:
> On Thu, Aug 17, 2023 at 4:29 PM Jeff Davis <pg...@j-davis.com> wrote:
>> On Wed, 2023-08-09 at 17:14 -0700, Peter Geoghegan wrote:
>>> + Index quals are better than equivalent index filters because bitmap
>>> index scans can only use index quals
>>
>> It seems there's consensus that:
>>
>> * Index Filters (Tomas's patch and the topic of this thread) are more
>> general, because they can work on any expression, e.g. 1/x, which can
>> throw an error if x=0.
>
> Agreed, but with one small caveat: they're not more general when it
> comes to bitmap index scans, which seem like they'll only ever be able
> to support index quals. But AFAICT that's the one and only exception.
>
Yeah. Some conditions are never gonna be translated into proper index
quals, either because it's not really possible or because no one just
implemented that.
FWIW it's not immediately obvious to me if bitmap index scans are unable
to support index filters because of some inherent limitation, or whether
it's something we could implement. Some index AMs simply can't support
index filters, because they don't know the "full" index tuple tuple
(e.g. GIN), but maybe other AMs could support that ...
>> * Index quals are more optimizable, because such operators are not
>> supposed to throw errors or have side effects, so we can evaluate them
>> before determining visibility.
>
> Right. Though there is a second reason: the index AM can use them to
> navigate the index more efficiently with true index quals. At least in
> the case of SAOPs, and perhaps in other cases in the future.
>
>> I wouldn't describe one as "better" than the other, but I assume you
>> meant "more optimizable".
>
> The use of the term "better" is actually descriptive of Tomas' patch.
> It is structured in a way that conditions the use of index filters on
> the absence of equivalent index quals for eligible/indexed clauses. So
> it really does make sense to think of it as a "qual hierarchy" (to use
> Tomas' term).
>
> It's also true that it will always be fundamentally impossible to use
> index quals for a significant proportion of all clause types, so
> "better when feasible at all" is slightly more accurate.
>
Yeah, I agree with all of this too. I think that in all practical cases
an index qual is strictly better than an index filter, for exactly these
reasons.
>> Is there any part of the design here that's preventing this patch from
>> moving forward? If not, what are the TODOs for the current patch, or is
>> it just waiting on more review?
>
> Practically speaking, I have no objections to moving ahead with this.
> I believe that the general structure of the patch makes sense, and I
> don't expect Tomas to do anything that wasn't already expected before
> I chimed in.
>
I agree the general idea is sound. The main "problem" in the original
patch was about costing changes and the implied risk of picking the
wrong plan (instead of e.g. a bitmap index scan). That's pretty much
what the "risk" in example (4) in the principles.txt is about, AFAICS.
I think the consensus is that if we don't change the cost, this issue
mostly goes away - we won't pick index scan in cases where we'd pick a
different plan without the patch, and for index scans it's (almost)
always correct to replace "table filter" with "index filter".
If that issue goes away, I think there are two smaller ones - picking
which conditions to promote to index filters, and actually tweaking the
index scan code to do that.
The first issue seems similar to the costing issue - in fact, it really
is a costing problem. The goal is to minimize the amount of work needed
to evaluate the conditions on all rows, and if we knew selectivity+cost
for each condition, it'd be a fairly simple matter. But in practice we
don't know this - the selectivity estimates are rather shaky (and doubly
so for correlated conditions), and the procedure cost estimates are
quite crude too ...
The risk is we might move "forward" very expensive condition. Imagine a
condition with procost=1000000, which would normally be evaluated last
after all other clauses. But if we promote it to an index filter, that'd
no longer be true.
What Jeff proposed last week was roughly this:
- find min(procost) for clauses that can't be index filters
- consider all clauses with procost <= min(procost) as index filters
But after looking at this I realized order_qual_clauses() does a very
similar thing for regular clauses, and the proposed logic contradicts
the heuristics order_qual_clauses() a bit.
In particular, order_qual_clauses() orders the clauses by procost, but
then it's careful not to reorder the clauses with the same procost. With
the proposed heuristics, that'd change - the clauses might get reordered
in various ways, possibly with procost values [1, 10, 100, 1, 10, 100]
or something like that.
Not sure if we want to relax the heuristics like this. There's also the
practical issue that order_qual_clauses() gets called quite late, long
after the index filters were built. And it also considers security
levels, which I ignored until now.
But now that I think about it, with the original patch it had to be
decided when building the path, because that's when the costing happens.
With the costing changes removed, we can do probably do that much later
while creating the plan, after order_qual_clauses() does all the work.
We could walk the qpqual list, and stop on the first element that can't
be treated as index filter. That'd also handle the security levels, and
it'd *almost* do what Jeff proposed, I think.
The main difference is that it'd not reorder clauses with the same
procost. With multiple clauses with the same procost, it'd depend on
which one happens to be first in the list, which AFAIK depends on the
order of conditions in the query. That may be a bit surprising, I guess,
because it seems natural that in a declarative language this should not
really matter. Yes, we already do that, but it probably does not have
huge impact. If it affects whether a condition will be treated as an
index filter, the differences are likely to be much more significant.
(FWIW this reminds me the issues we had with GROUP BY optimization,
which ultimately got reverted because the reordering was considered too
risky. I'm afraid we might end in the same situation ...)
As for the other issue, that's more about how to deal with the various
quals in the generic scan code. Imagine we have multiple conditions, and
and some of them can be treated as index filters. So for example we may
end up with this:
quals = [A, B, C, D]
index-filters = [A, B]
And now a tuple can fall into three categories:
a) all-visible=false, i.e. can't use index filter => quals
b) all-visible=true, and index-filters evaluates to false
c) all-visible=true, and index-filters evaluates to true
The first two cases are fine, but for (c) we need to also evaluate the
remaining non-filter quals [C, D]. We may build such list, and the 0002
patch does that, and shows the list in explain. But where/how would we
evaluate it? The code in execScan.c uses the ScanState->ps.qual, which
is set to [A,B,C,D]. Which is correct, but it does more work than
strictly necessary - the index filters are evaluated again. We can't
easily change that to [C,D] -> that would break the (a) case. Which
quals are evaluated on the heap tuple depends on visibility map and on
the index-filter result.
I think there's two options. First, we may accept that some of index
filters may get evaluated twice. That's not great, because it increases
the risk of regressions. Second, we could disable ScanState->ps.quals if
there are index filters, and just do all the work info nodeIndexscan.
But that seems quite ugly - in a way, the code already does that, which
is where the two loops
while (true)
{
for (;;)
{
...
}
}
come from. I was hoping to simplify / get rid of this, not making it do
even more stuff ... :-(
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL CompanyFrom 6719b7c9351244b488d65f39224c5b7f04d55351 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Tue, 19 Dec 2023 12:13:51 +0100
Subject: [PATCH v20231219 1/2] 20230716
---
src/backend/commands/explain.c | 2 +
src/backend/executor/nodeIndexscan.c | 258 +++++++++++++++++-
src/backend/optimizer/path/costsize.c | 23 ++
src/backend/optimizer/path/indxpath.c | 190 +++++++++++--
src/backend/optimizer/plan/createplan.c | 80 ++++++
src/backend/optimizer/plan/planner.c | 2 +-
src/backend/optimizer/plan/setrefs.c | 6 +
src/backend/optimizer/util/pathnode.c | 2 +
src/backend/utils/misc/guc_tables.c | 10 +
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/nodes/execnodes.h | 7 +
src/include/nodes/pathnodes.h | 1 +
src/include/nodes/plannodes.h | 2 +
src/include/optimizer/cost.h | 1 +
src/include/optimizer/pathnode.h | 1 +
src/test/regress/expected/create_index.out | 19 +-
src/test/regress/expected/sysviews.out | 3 +-
src/test/regress/expected/updatable_views.out | 12 +-
18 files changed, 580 insertions(+), 40 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index f1d71bc54e8..a30708dec9e 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1786,6 +1786,8 @@ ExplainNode(PlanState *planstate, List *ancestors,
case T_IndexScan:
show_scan_qual(((IndexScan *) plan)->indexqualorig,
"Index Cond", planstate, ancestors, es);
+ show_scan_qual(((IndexScan *) plan)->indexfiltersorig,
+ "Index Filter", planstate, ancestors, es);
if (((IndexScan *) plan)->indexqualorig)
show_instrumentation_count("Rows Removed by Index Recheck", 2,
planstate, es);
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 14b9c00217a..1ce8a7c19f3 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -32,6 +32,8 @@
#include "access/nbtree.h"
#include "access/relscan.h"
#include "access/tableam.h"
+#include "access/visibilitymap.h"
+#include "catalog/index.h"
#include "catalog/pg_am.h"
#include "executor/execdebug.h"
#include "executor/nodeIndexscan.h"
@@ -69,6 +71,10 @@ static void reorderqueue_push(IndexScanState *node, TupleTableSlot *slot,
Datum *orderbyvals, bool *orderbynulls);
static HeapTuple reorderqueue_pop(IndexScanState *node);
+static void StoreIndexTuple(TupleTableSlot *slot, IndexTuple itup,
+ TupleDesc itupdesc, IndexInfo *iinfo);
+static void StoreHeapTuple(TupleTableSlot *slot, HeapTuple tup,
+ TupleDesc tupdesc, IndexInfo *iinfo);
/* ----------------------------------------------------------------
* IndexNext
@@ -115,6 +121,13 @@ IndexNext(IndexScanState *node)
node->iss_ScanDesc = scandesc;
+ /* Set it up for index-only filters, if there are any. */
+ if (node->indexfilters != NULL)
+ {
+ node->iss_ScanDesc->xs_want_itup = true;
+ node->iss_VMBuffer = InvalidBuffer;
+ }
+
/*
* If no run-time keys to calculate or they are ready, go ahead and
* pass the scankeys to the index AM.
@@ -127,11 +140,150 @@ IndexNext(IndexScanState *node)
/*
* ok, now that we have what we need, fetch the next tuple.
+ *
+ * XXX maybe we should invent something like index_getnext_tid/index_getnext_slot
+ * that would allow doing this in a more readable / coherent way.
*/
- while (index_getnext_slot(scandesc, direction, slot))
+ while (true)
{
+ bool has_index_tuple = false;
+ bool filter_checked;
+
CHECK_FOR_INTERRUPTS();
+ /*
+ * XXX code from index_getnext_slot(), but we need to inject stuff between
+ * the index_getnext_tid() and index_fetch_heap(), so we do it here
+ */
+ for (;;)
+ {
+ filter_checked = false;
+
+ if (!scandesc->xs_heap_continue)
+ {
+ ItemPointer tid;
+
+ /* Time to fetch the next TID from the index */
+ tid = index_getnext_tid(scandesc, direction);
+
+ /* If we're out of index entries, we're done */
+ if (tid == NULL)
+ break;
+
+ Assert(ItemPointerEquals(tid, &scandesc->xs_heaptid));
+ }
+
+ /* Make sure we have a valid item pointer. */
+ Assert(ItemPointerIsValid(&scandesc->xs_heaptid));
+
+ /*
+ * If there are index clauses, try to evaluate the filter on the index
+ * tuple first, and only when it fails try the index tuple.
+ *
+ * https://www.postgresql.org/message-id/N1xaIrU29uk5YxLyW55MGk5fz9s6V2FNtj54JRaVlFbPixD5z8sJ07Ite5CvbWwik8ZvDG07oSTN-usENLVMq2UAcizVTEd5b-o16ZGDIIU%3D%40yamlcoder.me
+ */
+ if (node->indexfilters != NULL)
+ {
+ ItemPointer tid = &scandesc->xs_heaptid;
+
+ /*
+ * XXX see nodeIndexonlyscan.c, but inverse - we only do this when
+ * we can check some filters on the index tuple.
+ */
+ if (VM_ALL_VISIBLE(scandesc->heapRelation,
+ ItemPointerGetBlockNumber(tid),
+ &node->iss_VMBuffer))
+ {
+ /*
+ * Only MVCC snapshots are supported here, so there should be no
+ * need to keep following the HOT chain once a visible entry has
+ * been found. If we did want to allow that, we'd need to keep
+ * more state to remember not to call index_getnext_tid next time.
+ *
+ * XXX Is there a place where we can decide whether to consider
+ * this optimization, based on which type of snapshot we're going
+ * to use? And disable it for non-MVCC ones? That'd mean this
+ * error can't really happen here. Or how can we even get this
+ * error now?
+ */
+ if (scandesc->xs_heap_continue)
+ elog(ERROR, "non-MVCC snapshots are not supported with index filters");
+
+ /*
+ * Fill the scan tuple slot with data from the index. This might be
+ * provided in either HeapTuple or IndexTuple format. Conceivably an
+ * index AM might fill both fields, in which case we prefer the heap
+ * format, since it's probably a bit cheaper to fill a slot from.
+ */
+ if (scandesc->xs_hitup)
+ {
+ StoreHeapTuple(node->iss_TableSlot, scandesc->xs_hitup,
+ scandesc->xs_hitupdesc, node->iss_IndexInfo);
+ }
+ else if (scandesc->xs_itup)
+ StoreIndexTuple(node->iss_TableSlot, scandesc->xs_itup,
+ scandesc->xs_itupdesc, node->iss_IndexInfo);
+ else
+ elog(ERROR, "no data returned for index-only scan");
+
+ /* run the expressions
+ *
+ * XXX This does not work, because for some indexes the index key type
+ * may differ from the table attribute type. This happens when the
+ * (pg_opclass.opcintype != opckeytype). There's about 10 such built-in
+ * opclasses, e.g. name_ops creates cstring on name columns. A good
+ * example is pg_namespace.nspname, with the index on nspname causing
+ * trouble for \dT.
+ *
+ * One option would be to tweak the type for the Var, but I'm not sure
+ * that's actually correct. Surely some of the types may not be binary
+ * compatible (e.g. jsonb_ops has opcintype=jsonb and opckeytype=text).
+ * So maybe we should just disable index-only filters for such cases.
+ */
+ econtext->ecxt_scantuple = node->iss_TableSlot;
+
+ /* check the filters pushed to the index-tuple level */
+ if (!ExecQual(node->indexfilters, econtext))
+ {
+ InstrCountFiltered2(node, 1);
+ continue;
+ }
+
+ /* remember we already checked the filter */
+ filter_checked = true;
+ }
+ }
+
+ /*
+ * Fetch the next (or only) visible heap tuple for this index entry.
+ * If we don't find anything, loop around and grab the next TID from
+ * the index.
+ */
+ if (index_fetch_heap(scandesc, slot))
+ {
+ has_index_tuple = true;
+ break;
+ }
+ }
+
+ if (!has_index_tuple)
+ break;
+
+ /*
+ * If we didn't manage to check the filter on the index tuple (because
+ * of the page not being all-visible, etc.), do that now on the heap
+ * tuple.
+ */
+ if (!filter_checked)
+ {
+ econtext->ecxt_scantuple = slot;
+ if (!ExecQual(node->indexfiltersorig, econtext))
+ {
+ InstrCountFiltered2(node, 1);
+ continue;
+ }
+ }
+
/*
* If the index was lossy, we have to recheck the index quals using
* the fetched tuple.
@@ -794,6 +946,13 @@ ExecEndIndexScan(IndexScanState *node)
indexRelationDesc = node->iss_RelationDesc;
indexScanDesc = node->iss_ScanDesc;
+ /* Release VM buffer pin, if any. */
+ if (node->iss_VMBuffer != InvalidBuffer)
+ {
+ ReleaseBuffer(node->iss_VMBuffer);
+ node->iss_VMBuffer = InvalidBuffer;
+ }
+
/*
* close the index relation (no-op if we didn't open it)
*/
@@ -940,6 +1099,10 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
ExecInitQual(node->scan.plan.qual, (PlanState *) indexstate);
indexstate->indexqualorig =
ExecInitQual(node->indexqualorig, (PlanState *) indexstate);
+ indexstate->indexfilters =
+ ExecInitQual(node->indexfilters, (PlanState *) indexstate);
+ indexstate->indexfiltersorig =
+ ExecInitQual(node->indexfiltersorig, (PlanState *) indexstate);
indexstate->indexorderbyorig =
ExecInitExprList(node->indexorderbyorig, (PlanState *) indexstate);
@@ -955,6 +1118,19 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
lockmode = exec_rt_fetch(node->scan.scanrelid, estate)->rellockmode;
indexstate->iss_RelationDesc = index_open(node->indexid, lockmode);
+ /*
+ * We need another slot, in a format that's suitable for the table AM, for
+ * when we need to evaluate index-only filter on data from index tuple.
+ *
+ * XXX Maybe this could use the scan tuple slot?
+ */
+ indexstate->iss_TableSlot =
+ ExecAllocTableSlot(&estate->es_tupleTable,
+ RelationGetDescr(currentRelation),
+ table_slot_callbacks(currentRelation));
+
+ indexstate->iss_IndexInfo = BuildIndexInfo(indexstate->iss_RelationDesc);
+
/*
* Initialize index-specific scan state
*/
@@ -1728,3 +1904,83 @@ ExecIndexScanInitializeWorker(IndexScanState *node,
node->iss_ScanKeys, node->iss_NumScanKeys,
node->iss_OrderByKeys, node->iss_NumOrderByKeys);
}
+
+/*
+ * StoreIndexTuple
+ * Fill the slot with data from the index tuple.
+ *
+ * At some point this might be generally-useful functionality, but
+ * right now we don't need it elsewhere.
+ */
+static void
+StoreIndexTuple(TupleTableSlot *slot, IndexTuple itup, TupleDesc itupdesc, IndexInfo *iinfo)
+{
+ bool *isnull;
+ Datum *values;
+
+ isnull = palloc0(itupdesc->natts * sizeof(bool));
+ values = palloc0(itupdesc->natts * sizeof(Datum));
+
+ /* expand the index tuple */
+ index_deform_tuple(itup, itupdesc, values, isnull);
+
+ /* now fill the values from the index tuple into the table slot */
+ ExecClearTuple(slot);
+
+ for (int i = 0; i < iinfo->ii_NumIndexAttrs; i++)
+ {
+ AttrNumber attnum = iinfo->ii_IndexAttrNumbers[i];
+
+ /* skip expressions */
+ if (attnum > 0)
+ {
+ slot->tts_isnull[attnum - 1] = isnull[i];
+ slot->tts_values[attnum - 1] = values[i];
+ }
+ }
+
+ pfree(values);
+ pfree(isnull);
+
+ ExecStoreVirtualTuple(slot);
+}
+
+/*
+ * StoreHeapTuple
+ * Fill the slot with data from the heap tuple (from the index).
+ *
+ * At some point this might be generally-useful functionality, but
+ * right now we don't need it elsewhere.
+ */
+static void
+StoreHeapTuple(TupleTableSlot *slot, HeapTuple tup, TupleDesc tupdesc, IndexInfo *iinfo)
+{
+ bool *isnull;
+ Datum *values;
+
+ isnull = palloc0(tupdesc->natts * sizeof(bool));
+ values = palloc0(tupdesc->natts * sizeof(Datum));
+
+ /* expand the index tuple */
+ heap_deform_tuple(tup, tupdesc, values, isnull);
+
+ /* now fill the values from the index tuple into the table slot */
+ ExecClearTuple(slot);
+
+ for (int i = 0; i < iinfo->ii_NumIndexAttrs; i++)
+ {
+ AttrNumber attnum = iinfo->ii_IndexAttrNumbers[i];
+
+ /* skip expressions */
+ if (attnum > 0)
+ {
+ slot->tts_isnull[attnum - 1] = isnull[i];
+ slot->tts_values[attnum - 1] = values[i];
+ }
+ }
+
+ pfree(values);
+ pfree(isnull);
+
+ ExecStoreVirtualTuple(slot);
+}
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 22635d29270..fef7d62dab8 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -135,6 +135,7 @@ int max_parallel_workers_per_gather = 2;
bool enable_seqscan = true;
bool enable_indexscan = true;
bool enable_indexonlyscan = true;
+bool enable_indexonlyfilter = true;
bool enable_bitmapscan = true;
bool enable_tidscan = true;
bool enable_sort = true;
@@ -636,6 +637,28 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count,
/* estimate number of main-table tuples fetched */
tuples_fetched = clamp_row_est(indexSelectivity * baserel->tuples);
+ /*
+ * If some filters can be evaluated on the index tuple, account for that.
+ * We need to scan all tuples from pages that are not all-visible, and
+ * for the remaining tuples we fetch those not eliminated by the filter.
+ *
+ * XXX Does this need to worry about path->path.param_info?
+ *
+ * XXX All of this seems overly manual / ad-hoc, surely there's a place
+ * where we already do this in a more elegant manner?
+ */
+ if (path->indexfilters != NIL)
+ {
+ Selectivity sel;
+
+ sel = clauselist_selectivity(root, path->indexfilters, baserel->relid,
+ JOIN_INNER, NULL);
+
+ tuples_fetched *= (1.0 - baserel->allvisfrac) + (baserel->allvisfrac) * sel;
+
+ tuples_fetched = clamp_row_est(tuples_fetched);
+ }
+
/* fetch estimated page costs for tablespace containing table */
get_tablespace_page_costs(baserel->reltablespace,
&spc_random_page_cost,
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 03a5fbdc6dc..aec05e1a087 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -101,9 +101,11 @@ static bool eclass_already_used(EquivalenceClass *parent_ec, Relids oldrelids,
List *indexjoinclauses);
static void get_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
- List **bitindexpaths);
+ List *filters, List **bitindexpaths);
static List *build_index_paths(PlannerInfo *root, RelOptInfo *rel,
- IndexOptInfo *index, IndexClauseSet *clauses,
+ IndexOptInfo *index,
+ IndexClauseSet *clauses,
+ List *filters,
bool useful_predicate,
ScanTypeControl scantype,
bool *skip_nonnative_saop,
@@ -124,6 +126,7 @@ static PathClauseUsage *classify_index_clause_usage(Path *path,
static void find_indexpath_quals(Path *bitmapqual, List **quals, List **preds);
static int find_list_position(Node *node, List **nodelist);
static bool check_index_only(RelOptInfo *rel, IndexOptInfo *index);
+static bool check_index_filter(RelOptInfo *rel, IndexOptInfo *index, Node *clause);
static double get_loop_count(PlannerInfo *root, Index cur_relid, Relids outer_relids);
static double adjust_rowcount_for_semijoins(PlannerInfo *root,
Index cur_relid,
@@ -132,7 +135,8 @@ static double adjust_rowcount_for_semijoins(PlannerInfo *root,
static double approximate_joinrel_size(PlannerInfo *root, Relids relids);
static void match_restriction_clauses_to_index(PlannerInfo *root,
IndexOptInfo *index,
- IndexClauseSet *clauseset);
+ IndexClauseSet *clauseset,
+ List **filters);
static void match_join_clauses_to_index(PlannerInfo *root,
RelOptInfo *rel, IndexOptInfo *index,
IndexClauseSet *clauseset,
@@ -143,15 +147,20 @@ static void match_eclass_clauses_to_index(PlannerInfo *root,
static void match_clauses_to_index(PlannerInfo *root,
List *clauses,
IndexOptInfo *index,
- IndexClauseSet *clauseset);
+ IndexClauseSet *clauseset,
+ List **filters);
static void match_clause_to_index(PlannerInfo *root,
RestrictInfo *rinfo,
IndexOptInfo *index,
- IndexClauseSet *clauseset);
+ IndexClauseSet *clauseset,
+ List **filters);
static IndexClause *match_clause_to_indexcol(PlannerInfo *root,
RestrictInfo *rinfo,
int indexcol,
IndexOptInfo *index);
+static RestrictInfo *match_filter_to_index(PlannerInfo *root,
+ RestrictInfo *rinfo,
+ IndexOptInfo *index);
static bool IsBooleanOpfamily(Oid opfamily);
static IndexClause *match_boolean_index_clause(PlannerInfo *root,
RestrictInfo *rinfo,
@@ -241,6 +250,7 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
IndexClauseSet rclauseset;
IndexClauseSet jclauseset;
IndexClauseSet eclauseset;
+ List *rfilters;
ListCell *lc;
/* Skip the whole mess if no indexes */
@@ -270,14 +280,15 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
* Identify the restriction clauses that can match the index.
*/
MemSet(&rclauseset, 0, sizeof(rclauseset));
- match_restriction_clauses_to_index(root, index, &rclauseset);
+ rfilters = NIL;
+ match_restriction_clauses_to_index(root, index, &rclauseset, &rfilters);
/*
* Build index paths from the restriction clauses. These will be
* non-parameterized paths. Plain paths go directly to add_path(),
* bitmap paths are added to bitindexpaths to be handled below.
*/
- get_index_paths(root, rel, index, &rclauseset,
+ get_index_paths(root, rel, index, &rclauseset, rfilters,
&bitindexpaths);
/*
@@ -301,6 +312,8 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
/*
* If we found any plain or eclass join clauses, build parameterized
* index paths using them.
+ *
+ * XXX Maybe pass the filters too?
*/
if (jclauseset.nonempty || eclauseset.nonempty)
consider_index_join_clauses(root, rel, index,
@@ -662,7 +675,7 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
Assert(clauseset.nonempty);
/* Build index path(s) using the collected set of clauses */
- get_index_paths(root, rel, index, &clauseset, bitindexpaths);
+ get_index_paths(root, rel, index, &clauseset, NULL, bitindexpaths);
/*
* Remember we considered paths for this set of relids.
@@ -712,7 +725,7 @@ eclass_already_used(EquivalenceClass *parent_ec, Relids oldrelids,
static void
get_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
- List **bitindexpaths)
+ List *filters, List **bitindexpaths)
{
List *indexpaths;
bool skip_nonnative_saop = false;
@@ -726,7 +739,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
* paths if possible).
*/
indexpaths = build_index_paths(root, rel,
- index, clauses,
+ index, clauses, filters,
index->predOK,
ST_ANYSCAN,
&skip_nonnative_saop,
@@ -741,7 +754,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
indexpaths = list_concat(indexpaths,
build_index_paths(root, rel,
- index, clauses,
+ index, clauses, filters,
index->predOK,
ST_ANYSCAN,
&skip_nonnative_saop,
@@ -781,7 +794,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
if (skip_nonnative_saop)
{
indexpaths = build_index_paths(root, rel,
- index, clauses,
+ index, clauses, filters,
false,
ST_BITMAPSCAN,
NULL,
@@ -833,7 +846,9 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
*/
static List *
build_index_paths(PlannerInfo *root, RelOptInfo *rel,
- IndexOptInfo *index, IndexClauseSet *clauses,
+ IndexOptInfo *index,
+ IndexClauseSet *clauses,
+ List *filters,
bool useful_predicate,
ScanTypeControl scantype,
bool *skip_nonnative_saop,
@@ -842,6 +857,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
List *result = NIL;
IndexPath *ipath;
List *index_clauses;
+ List *index_filters;
Relids outer_relids;
double loop_count;
List *orderbyclauses;
@@ -947,6 +963,26 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
return NIL;
}
+ /*
+ * If we have index-only filters, combine the clauses into a simple list and
+ * add the relids to outer relids.
+ */
+ index_filters = NIL;
+ if (filters)
+ {
+ ListCell *lc;
+
+ foreach(lc, filters)
+ {
+ RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
+
+ /* OK to include this clause (as a filter) */
+ index_filters = lappend(index_filters, rinfo);
+ outer_relids = bms_add_members(outer_relids,
+ rinfo->clause_relids);
+ }
+ }
+
/* We do not want the index's rel itself listed in outer_relids */
outer_relids = bms_del_member(outer_relids, rel->relid);
@@ -1015,6 +1051,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
ipath = create_index_path(root, index,
index_clauses,
+ index_filters,
orderbyclauses,
orderbyclausecols,
useful_pathkeys,
@@ -1035,6 +1072,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
ipath = create_index_path(root, index,
index_clauses,
+ index_filters,
orderbyclauses,
orderbyclausecols,
useful_pathkeys,
@@ -1068,6 +1106,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
ipath = create_index_path(root, index,
index_clauses,
+ index_filters,
NIL,
NIL,
useful_pathkeys,
@@ -1085,6 +1124,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
ipath = create_index_path(root, index,
index_clauses,
+ index_filters,
NIL,
NIL,
useful_pathkeys,
@@ -1147,6 +1187,7 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
{
IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
IndexClauseSet clauseset;
+ List *filters;
List *indexpaths;
bool useful_predicate;
@@ -1191,11 +1232,14 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
* Identify the restriction clauses that can match the index.
*/
MemSet(&clauseset, 0, sizeof(clauseset));
- match_clauses_to_index(root, clauses, index, &clauseset);
+ filters = NIL;
+ match_clauses_to_index(root, clauses, index, &clauseset, &filters);
/*
* If no matches so far, and the index predicate isn't useful, we
* don't want it.
+ *
+ * XXX Maybe this should check the filterset too?
*/
if (!clauseset.nonempty && !useful_predicate)
continue;
@@ -1203,13 +1247,13 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
/*
* Add "other" restriction clauses to the clauseset.
*/
- match_clauses_to_index(root, other_clauses, index, &clauseset);
+ match_clauses_to_index(root, other_clauses, index, &clauseset, &filters);
/*
* Construct paths if possible.
*/
indexpaths = build_index_paths(root, rel,
- index, &clauseset,
+ index, &clauseset, filters,
useful_predicate,
ST_BITMAPSCAN,
NULL,
@@ -1853,6 +1897,62 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
return result;
}
+/*
+ * check_index_filter
+ * Determine whether a clause can be executed directly on the index tuple.
+ */
+static bool
+check_index_filter(RelOptInfo *rel, IndexOptInfo *index, Node *clause)
+{
+ bool result;
+ Bitmapset *attrs_used = NULL;
+ Bitmapset *index_canreturn_attrs = NULL;
+ int i;
+
+ /* Index-only scans must be enabled */
+ if (!enable_indexonlyfilter)
+ return false;
+
+ /*
+ * Check that all needed attributes of the relation are available from the
+ * index.
+ */
+
+ /*
+ * First, identify all the attributes needed by the clause.
+ */
+ pull_varattnos(clause, rel->relid, &attrs_used);
+
+ /*
+ * Construct a bitmapset of columns that the index can return back in an
+ * index-only scan.
+ */
+ for (i = 0; i < index->ncolumns; i++)
+ {
+ int attno = index->indexkeys[i];
+
+ /*
+ * For the moment, we just ignore index expressions. It might be nice
+ * to do something with them, later.
+ */
+ if (attno == 0)
+ continue;
+
+ if (index->canreturn[i])
+ index_canreturn_attrs =
+ bms_add_member(index_canreturn_attrs,
+ attno - FirstLowInvalidHeapAttributeNumber);
+ }
+
+ /* Do we have all the necessary attributes? */
+ result = bms_is_subset(attrs_used, index_canreturn_attrs);
+
+ bms_free(attrs_used);
+ bms_free(index_canreturn_attrs);
+
+ return result;
+}
+
/*
* get_loop_count
* Choose the loop count estimate to use for costing a parameterized path
@@ -2021,10 +2121,11 @@ approximate_joinrel_size(PlannerInfo *root, Relids relids)
static void
match_restriction_clauses_to_index(PlannerInfo *root,
IndexOptInfo *index,
- IndexClauseSet *clauseset)
+ IndexClauseSet *clauseset,
+ List **filters)
{
/* We can ignore clauses that are implied by the index predicate */
- match_clauses_to_index(root, index->indrestrictinfo, index, clauseset);
+ match_clauses_to_index(root, index->indrestrictinfo, index, clauseset, filters);
}
/*
@@ -2032,6 +2133,8 @@ match_restriction_clauses_to_index(PlannerInfo *root,
* Identify join clauses for the rel that match the index.
* Matching clauses are added to *clauseset.
* Also, add any potentially usable join OR clauses to *joinorclauses.
+ *
+ * FIXME Maybe this should fill the filterset too?
*/
static void
match_join_clauses_to_index(PlannerInfo *root,
@@ -2054,7 +2157,7 @@ match_join_clauses_to_index(PlannerInfo *root,
if (restriction_is_or_clause(rinfo))
*joinorclauses = lappend(*joinorclauses, rinfo);
else
- match_clause_to_index(root, rinfo, index, clauseset);
+ match_clause_to_index(root, rinfo, index, clauseset, NULL);
}
}
@@ -2062,6 +2165,8 @@ match_join_clauses_to_index(PlannerInfo *root,
* match_eclass_clauses_to_index
* Identify EquivalenceClass join clauses for the rel that match the index.
* Matching clauses are added to *clauseset.
+ *
+ * XXX Maybe this should fill the filterset too?
*/
static void
match_eclass_clauses_to_index(PlannerInfo *root, IndexOptInfo *index,
@@ -2092,7 +2197,7 @@ match_eclass_clauses_to_index(PlannerInfo *root, IndexOptInfo *index,
* since for non-btree indexes the EC's equality operators might not
* be in the index opclass (cf ec_member_matches_indexcol).
*/
- match_clauses_to_index(root, clauses, index, clauseset);
+ match_clauses_to_index(root, clauses, index, clauseset, NULL);
}
}
@@ -2105,7 +2210,8 @@ static void
match_clauses_to_index(PlannerInfo *root,
List *clauses,
IndexOptInfo *index,
- IndexClauseSet *clauseset)
+ IndexClauseSet *clauseset,
+ List **filters)
{
ListCell *lc;
@@ -2113,7 +2219,7 @@ match_clauses_to_index(PlannerInfo *root,
{
RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
- match_clause_to_index(root, rinfo, index, clauseset);
+ match_clause_to_index(root, rinfo, index, clauseset, filters);
}
}
@@ -2138,7 +2244,8 @@ static void
match_clause_to_index(PlannerInfo *root,
RestrictInfo *rinfo,
IndexOptInfo *index,
- IndexClauseSet *clauseset)
+ IndexClauseSet *clauseset,
+ List **filters)
{
int indexcol;
@@ -2187,6 +2294,20 @@ match_clause_to_index(PlannerInfo *root,
return;
}
}
+
+ /* if filterset is NULL, we're done */
+ if (!filters)
+ return;
+
+ /*
+ * We didn't record the clause as a regular index clause, so see if
+ * we can evaluate it as an index filter.
+ */
+ if ((rinfo = match_filter_to_index(root, rinfo, index)) != NULL)
+ {
+ /* FIXME maybe check/prevent duplicates, like above? */
+ *filters = lappend(*filters, rinfo);
+ }
}
/*
@@ -2322,6 +2443,29 @@ match_clause_to_indexcol(PlannerInfo *root,
return NULL;
}
+static RestrictInfo *
+match_filter_to_index(PlannerInfo *root,
+ RestrictInfo *rinfo,
+ IndexOptInfo *index)
+{
+ Expr *clause = rinfo->clause;
+
+ /*
+ * Historically this code has coped with NULL clauses. That's probably
+ * not possible anymore, but we might as well continue to cope.
+ */
+ if (clause == NULL)
+ return NULL;
+
+ /*
+ * Can the clause be evaluated only using the index tuple?
+ */
+ if (!check_index_filter(index->rel, index, (Node *) rinfo->clause))
+ return NULL;
+
+ return rinfo;
+}
+
/*
* IsBooleanOpfamily
* Detect whether an opfamily supports boolean equality as an operator.
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 34ca6d4ac21..cddc87d4fb8 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -166,10 +166,16 @@ static Node *replace_nestloop_params_mutator(Node *node, PlannerInfo *root);
static void fix_indexqual_references(PlannerInfo *root, IndexPath *index_path,
List **stripped_indexquals_p,
List **fixed_indexquals_p);
+static void fix_indexfilter_references(PlannerInfo *root, IndexPath *index_path,
+ List **stripped_indexfilters_p,
+ List **fixed_indexfilters_p);
static List *fix_indexorderby_references(PlannerInfo *root, IndexPath *index_path);
static Node *fix_indexqual_clause(PlannerInfo *root,
IndexOptInfo *index, int indexcol,
Node *clause, List *indexcolnos);
+static Node *fix_indexfilter_clause(PlannerInfo *root,
+ IndexOptInfo *index,
+ Node *clause);
static Node *fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol);
static List *get_switched_clauses(List *clauses, Relids outerrelids);
static List *order_qual_clauses(PlannerInfo *root, List *clauses);
@@ -182,6 +188,7 @@ static SampleScan *make_samplescan(List *qptlist, List *qpqual, Index scanrelid,
TableSampleClause *tsc);
static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid,
Oid indexid, List *indexqual, List *indexqualorig,
+ List *indexfilters, List *indexfiltersorig,
List *indexorderby, List *indexorderbyorig,
List *indexorderbyops,
ScanDirection indexscandir);
@@ -3013,7 +3020,9 @@ create_indexscan_plan(PlannerInfo *root,
Oid indexoid = indexinfo->indexoid;
List *qpqual;
List *stripped_indexquals;
+ List *stripped_indexfilters;
List *fixed_indexquals;
+ List *fixed_indexfilters;
List *fixed_indexorderbys;
List *indexorderbyops = NIL;
ListCell *l;
@@ -3035,6 +3044,16 @@ create_indexscan_plan(PlannerInfo *root,
&stripped_indexquals,
&fixed_indexquals);
+ /*
+ * Extract the index qual expressions (stripped of RestrictInfos) from the
+ * IndexClauses list, and prepare a copy with index Vars substituted for
+ * table Vars. (This step also does replace_nestloop_params on the
+ * fixed_indexquals.)
+ */
+ fix_indexfilter_references(root, best_path,
+ &stripped_indexfilters,
+ &fixed_indexfilters);
+
/*
* Likewise fix up index attr references in the ORDER BY expressions.
*/
@@ -3103,6 +3122,8 @@ create_indexscan_plan(PlannerInfo *root,
{
stripped_indexquals = (List *)
replace_nestloop_params(root, (Node *) stripped_indexquals);
+ stripped_indexfilters = (List *)
+ replace_nestloop_params(root, (Node *) stripped_indexfilters);
qpqual = (List *)
replace_nestloop_params(root, (Node *) qpqual);
indexorderbys = (List *)
@@ -3179,6 +3200,8 @@ create_indexscan_plan(PlannerInfo *root,
indexoid,
fixed_indexquals,
stripped_indexquals,
+ fixed_indexfilters,
+ stripped_indexfilters,
fixed_indexorderbys,
indexorderbys,
indexorderbyops,
@@ -5023,6 +5046,40 @@ fix_indexqual_references(PlannerInfo *root, IndexPath *index_path,
*fixed_indexquals_p = fixed_indexquals;
}
+/*
+ * fix_indexfilter_references
+ * Adjust indexfilter clauses to the form the executor's indexfilter
+ * machinery needs.
+ *
+ * XXX This does similar stuff to fix_indexqual_references does, except that it
+ * doesn't switch the Vars to point to the index attnum (we'll expand the index
+ * tuple into the heap tuple and run the expression on that).
+ */
+static void
+fix_indexfilter_references(PlannerInfo *root, IndexPath *index_path,
+ List **stripped_indexfilters_p, List **fixed_indexfilters_p)
+{
+ IndexOptInfo *index = index_path->indexinfo;
+ List *stripped_indexfilters;
+ List *fixed_indexfilters;
+ ListCell *lc;
+
+ stripped_indexfilters = fixed_indexfilters = NIL;
+
+ foreach(lc, index_path->indexfilters)
+ {
+ RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
+ Node *clause = (Node *) rinfo->clause;
+
+ stripped_indexfilters = lappend(stripped_indexfilters, clause);
+ clause = fix_indexfilter_clause(root, index, clause);
+ fixed_indexfilters = lappend(fixed_indexfilters, clause);
+ }
+
+ *stripped_indexfilters_p = stripped_indexfilters;
+ *fixed_indexfilters_p = fixed_indexfilters;
+}
+
/*
* fix_indexorderby_references
* Adjust indexorderby clauses to the form the executor's index
@@ -5121,6 +5178,25 @@ fix_indexqual_clause(PlannerInfo *root, IndexOptInfo *index, int indexcol,
return clause;
}
+/*
+ * fix_indexfilter_clause
+ * Convert a single indexqual clause to the form needed by the executor.
+ *
+ * We only replace nestloop params here. The Vars are left pointing to the
+ * table varno.
+ */
+static Node *
+fix_indexfilter_clause(PlannerInfo *root, IndexOptInfo *index, Node *clause)
+{
+ /*
+ * Replace any outer-relation variables with nestloop params.
+ *
+ * This also makes a copy of the clause, so it's safe to modify it
+ * in-place below (not done, actually).
+ */
+ return replace_nestloop_params(root, clause);
+}
+
/*
* fix_indexqual_operand
* Convert an indexqual expression to a Var referencing the index column.
@@ -5519,6 +5595,8 @@ make_indexscan(List *qptlist,
Oid indexid,
List *indexqual,
List *indexqualorig,
+ List *indexfilters,
+ List *indexfiltersorig,
List *indexorderby,
List *indexorderbyorig,
List *indexorderbyops,
@@ -5535,6 +5613,8 @@ make_indexscan(List *qptlist,
node->indexid = indexid;
node->indexqual = indexqual;
node->indexqualorig = indexqualorig;
+ node->indexfilters = indexfilters;
+ node->indexfiltersorig = indexfiltersorig;
node->indexorderby = indexorderby;
node->indexorderbyorig = indexorderbyorig;
node->indexorderbyops = indexorderbyops;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 6f45efde21d..39d7afc9f1f 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -6658,7 +6658,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid)
/* Estimate the cost of index scan */
indexScanPath = create_index_path(root, indexInfo,
- NIL, NIL, NIL, NIL,
+ NIL, NIL, NIL, NIL, NIL,
ForwardScanDirection, false,
NULL, 1.0, false);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 4bb68ac90e7..9d48de543ab 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -667,6 +667,12 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->indexqualorig =
fix_scan_list(root, splan->indexqualorig,
rtoffset, NUM_EXEC_QUAL(plan));
+ splan->indexfilters =
+ fix_scan_list(root, splan->indexfilters,
+ rtoffset, 1);
+ splan->indexfiltersorig =
+ fix_scan_list(root, splan->indexfiltersorig,
+ rtoffset, NUM_EXEC_QUAL(plan));
splan->indexorderby =
fix_scan_list(root, splan->indexorderby,
rtoffset, 1);
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 0b1d17b9d33..a6b44eed7e0 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -995,6 +995,7 @@ IndexPath *
create_index_path(PlannerInfo *root,
IndexOptInfo *index,
List *indexclauses,
+ List *indexfilters,
List *indexorderbys,
List *indexorderbycols,
List *pathkeys,
@@ -1019,6 +1020,7 @@ create_index_path(PlannerInfo *root,
pathnode->indexinfo = index;
pathnode->indexclauses = indexclauses;
+ pathnode->indexfilters = indexfilters;
pathnode->indexorderbys = indexorderbys;
pathnode->indexorderbycols = indexorderbycols;
pathnode->indexscandir = indexscandir;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index f7c9882f7c5..56b4584fa08 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -839,6 +839,16 @@ struct config_bool ConfigureNamesBool[] =
true,
NULL, NULL, NULL
},
+ {
+ {"enable_indexonlyfilter", PGC_USERSET, QUERY_TUNING_METHOD,
+ gettext_noop("Enables the planner's use of index to evaluate filters."),
+ NULL,
+ GUC_EXPLAIN
+ },
+ &enable_indexonlyfilter,
+ true,
+ NULL, NULL, NULL
+ },
{
{"enable_bitmapscan", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of bitmap-scan plans."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index cf9f283cfee..727a360d133 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -381,6 +381,7 @@
#enable_incremental_sort = on
#enable_indexscan = on
#enable_indexonlyscan = on
+#enable_indexonlyfilter = on
#enable_material = on
#enable_memoize = on
#enable_mergejoin = on
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 5d7f17dee07..6b50d3808a1 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1571,6 +1571,13 @@ typedef struct IndexScanState
Relation iss_RelationDesc;
struct IndexScanDescData *iss_ScanDesc;
+ /* index-only filters */
+ ExprState *indexfilters;
+ ExprState *indexfiltersorig;
+ TupleTableSlot *iss_TableSlot;
+ Buffer iss_VMBuffer;
+ IndexInfo *iss_IndexInfo;
+
/* These are needed for re-checking ORDER BY expr ordering */
pairingheap *iss_ReorderQueue;
bool iss_ReachedEnd;
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index ed85dc7414b..5311a75529d 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1677,6 +1677,7 @@ typedef struct IndexPath
Path path;
IndexOptInfo *indexinfo;
List *indexclauses;
+ List *indexfilters;
List *indexorderbys;
List *indexorderbycols;
ScanDirection indexscandir;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index d40af8e59fe..0b0909e209f 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -450,6 +450,8 @@ typedef struct IndexScan
Oid indexid; /* OID of index to scan */
List *indexqual; /* list of index quals (usually OpExprs) */
List *indexqualorig; /* the same in original form */
+ List *indexfilters; /* quals for included columns */
+ List *indexfiltersorig; /* the same in original form */
List *indexorderby; /* list of index ORDER BY exprs */
List *indexorderbyorig; /* the same in original form */
List *indexorderbyops; /* OIDs of sort ops for ORDER BY exprs */
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index 366adbfc39e..7e27c044aa3 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -52,6 +52,7 @@ extern PGDLLIMPORT int max_parallel_workers_per_gather;
extern PGDLLIMPORT bool enable_seqscan;
extern PGDLLIMPORT bool enable_indexscan;
extern PGDLLIMPORT bool enable_indexonlyscan;
+extern PGDLLIMPORT bool enable_indexonlyfilter;
extern PGDLLIMPORT bool enable_bitmapscan;
extern PGDLLIMPORT bool enable_tidscan;
extern PGDLLIMPORT bool enable_sort;
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index 6e557bebc44..c9120f8f2e0 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -41,6 +41,7 @@ extern Path *create_samplescan_path(PlannerInfo *root, RelOptInfo *rel,
extern IndexPath *create_index_path(PlannerInfo *root,
IndexOptInfo *index,
List *indexclauses,
+ List *indexfilters,
List *indexorderbys,
List *indexorderbycols,
List *pathkeys,
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 446cfa678b7..15c13faae6b 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1838,18 +1838,13 @@ DROP TABLE onek_with_null;
EXPLAIN (COSTS OFF)
SELECT * FROM tenk1
WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
- QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------
- Bitmap Heap Scan on tenk1
- Recheck Cond: (((thousand = 42) AND (tenthous = 1)) OR ((thousand = 42) AND (tenthous = 3)) OR ((thousand = 42) AND (tenthous = 42)))
- -> BitmapOr
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: ((thousand = 42) AND (tenthous = 1))
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: ((thousand = 42) AND (tenthous = 3))
- -> Bitmap Index Scan on tenk1_thous_tenthous
- Index Cond: ((thousand = 42) AND (tenthous = 42))
-(9 rows)
+ QUERY PLAN
+-----------------------------------------------------------------------
+ Index Scan using tenk1_thous_tenthous on tenk1
+ Index Cond: (thousand = 42)
+ Index Filter: ((tenthous = 1) OR (tenthous = 3) OR (tenthous = 42))
+ Filter: ((tenthous = 1) OR (tenthous = 3) OR (tenthous = 42))
+(4 rows)
SELECT * FROM tenk1
WHERE thousand = 42 AND (tenthous = 1 OR tenthous = 3 OR tenthous = 42);
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 271313ebf86..761866b9b92 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -117,6 +117,7 @@ select name, setting from pg_settings where name like 'enable%';
enable_hashagg | on
enable_hashjoin | on
enable_incremental_sort | on
+ enable_indexonlyfilter | on
enable_indexonlyscan | on
enable_indexscan | on
enable_material | on
@@ -133,7 +134,7 @@ select name, setting from pg_settings where name like 'enable%';
enable_seqscan | on
enable_sort | on
enable_tidscan | on
-(22 rows)
+(23 rows)
-- There are always wait event descriptions for various types.
select type, count(*) > 0 as ok FROM pg_wait_events
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index a73c1f90c4a..f29fcec8fc2 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2596,6 +2596,7 @@ UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6;
-> Index Scan using t1_a_idx on public.t1 t1_1
Output: t1_1.tableoid, t1_1.ctid
Index Cond: ((t1_1.a > 5) AND (t1_1.a < 7))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_1.a) AND (t1_1.a <> 6))
Filter: ((t1_1.a <> 6) AND (SubPlan 1) AND snoop(t1_1.a) AND leakproof(t1_1.a))
SubPlan 1
-> Append
@@ -2606,16 +2607,19 @@ UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6;
-> Index Scan using t11_a_idx on public.t11 t1_2
Output: t1_2.tableoid, t1_2.ctid
Index Cond: ((t1_2.a > 5) AND (t1_2.a < 7))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_2.a) AND (t1_2.a <> 6))
Filter: ((t1_2.a <> 6) AND (SubPlan 1) AND snoop(t1_2.a) AND leakproof(t1_2.a))
-> Index Scan using t12_a_idx on public.t12 t1_3
Output: t1_3.tableoid, t1_3.ctid
Index Cond: ((t1_3.a > 5) AND (t1_3.a < 7))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_3.a) AND (t1_3.a <> 6))
Filter: ((t1_3.a <> 6) AND (SubPlan 1) AND snoop(t1_3.a) AND leakproof(t1_3.a))
-> Index Scan using t111_a_idx on public.t111 t1_4
Output: t1_4.tableoid, t1_4.ctid
Index Cond: ((t1_4.a > 5) AND (t1_4.a < 7))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_4.a) AND (t1_4.a <> 6))
Filter: ((t1_4.a <> 6) AND (SubPlan 1) AND snoop(t1_4.a) AND leakproof(t1_4.a))
-(30 rows)
+(34 rows)
UPDATE v1 SET a=100 WHERE snoop(a) AND leakproof(a) AND a < 7 AND a != 6;
SELECT * FROM v1 WHERE a=100; -- Nothing should have been changed to 100
@@ -2643,6 +2647,7 @@ UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8;
-> Index Scan using t1_a_idx on public.t1 t1_1
Output: t1_1.a, t1_1.tableoid, t1_1.ctid
Index Cond: ((t1_1.a > 5) AND (t1_1.a = 8))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_1.a))
Filter: ((SubPlan 1) AND snoop(t1_1.a) AND leakproof(t1_1.a))
SubPlan 1
-> Append
@@ -2653,16 +2658,19 @@ UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8;
-> Index Scan using t11_a_idx on public.t11 t1_2
Output: t1_2.a, t1_2.tableoid, t1_2.ctid
Index Cond: ((t1_2.a > 5) AND (t1_2.a = 8))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_2.a))
Filter: ((SubPlan 1) AND snoop(t1_2.a) AND leakproof(t1_2.a))
-> Index Scan using t12_a_idx on public.t12 t1_3
Output: t1_3.a, t1_3.tableoid, t1_3.ctid
Index Cond: ((t1_3.a > 5) AND (t1_3.a = 8))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_3.a))
Filter: ((SubPlan 1) AND snoop(t1_3.a) AND leakproof(t1_3.a))
-> Index Scan using t111_a_idx on public.t111 t1_4
Output: t1_4.a, t1_4.tableoid, t1_4.ctid
Index Cond: ((t1_4.a > 5) AND (t1_4.a = 8))
+ Index Filter: ((SubPlan 1) AND leakproof(t1_4.a))
Filter: ((SubPlan 1) AND snoop(t1_4.a) AND leakproof(t1_4.a))
-(30 rows)
+(34 rows)
UPDATE v1 SET a=a+1 WHERE snoop(a) AND leakproof(a) AND a = 8;
NOTICE: snooped value: 8
--
2.41.0
From 49e94fce8e26c94475eea232c37c6e22c6837016 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@postgresql.org>
Date: Tue, 19 Dec 2023 13:59:07 +0100
Subject: [PATCH v20231219 2/2] reworks
---
src/backend/commands/explain.c | 4 +-
src/backend/executor/nodeIndexscan.c | 18 ++--
src/backend/optimizer/path/costsize.c | 22 -----
src/backend/optimizer/path/indxpath.c | 109 ++++++++++++------------
src/backend/optimizer/plan/createplan.c | 43 ++++++++--
src/backend/optimizer/plan/setrefs.c | 11 ++-
src/include/nodes/execnodes.h | 24 +++++-
src/include/nodes/plannodes.h | 16 +++-
8 files changed, 144 insertions(+), 103 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a30708dec9e..db5a8bd0436 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1786,8 +1786,10 @@ ExplainNode(PlanState *planstate, List *ancestors,
case T_IndexScan:
show_scan_qual(((IndexScan *) plan)->indexqualorig,
"Index Cond", planstate, ancestors, es);
- show_scan_qual(((IndexScan *) plan)->indexfiltersorig,
+ show_scan_qual(((IndexScan *) plan)->indexfilterorig,
"Index Filter", planstate, ancestors, es);
+ show_scan_qual(((IndexScan *) plan)->indexfilterqual,
+ "Index Filter (non-index)", planstate, ancestors, es);
if (((IndexScan *) plan)->indexqualorig)
show_instrumentation_count("Rows Removed by Index Recheck", 2,
planstate, es);
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 1ce8a7c19f3..4fb768741ae 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -122,7 +122,7 @@ IndexNext(IndexScanState *node)
node->iss_ScanDesc = scandesc;
/* Set it up for index-only filters, if there are any. */
- if (node->indexfilters != NULL)
+ if (node->indexfilter != NULL)
{
node->iss_ScanDesc->xs_want_itup = true;
node->iss_VMBuffer = InvalidBuffer;
@@ -182,7 +182,7 @@ IndexNext(IndexScanState *node)
*
* https://www.postgresql.org/message-id/N1xaIrU29uk5YxLyW55MGk5fz9s6V2FNtj54JRaVlFbPixD5z8sJ07Ite5CvbWwik8ZvDG07oSTN-usENLVMq2UAcizVTEd5b-o16ZGDIIU%3D%40yamlcoder.me
*/
- if (node->indexfilters != NULL)
+ if (node->indexfilter != NULL)
{
ItemPointer tid = &scandesc->xs_heaptid;
@@ -243,7 +243,7 @@ IndexNext(IndexScanState *node)
econtext->ecxt_scantuple = node->iss_TableSlot;
/* check the filters pushed to the index-tuple level */
- if (!ExecQual(node->indexfilters, econtext))
+ if (!ExecQual(node->indexfilter, econtext))
{
InstrCountFiltered2(node, 1);
continue;
@@ -277,7 +277,7 @@ IndexNext(IndexScanState *node)
if (!filter_checked)
{
econtext->ecxt_scantuple = slot;
- if (!ExecQual(node->indexfiltersorig, econtext))
+ if (!ExecQual(node->indexfilterorig, econtext))
{
InstrCountFiltered2(node, 1);
continue;
@@ -1099,10 +1099,12 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
ExecInitQual(node->scan.plan.qual, (PlanState *) indexstate);
indexstate->indexqualorig =
ExecInitQual(node->indexqualorig, (PlanState *) indexstate);
- indexstate->indexfilters =
- ExecInitQual(node->indexfilters, (PlanState *) indexstate);
- indexstate->indexfiltersorig =
- ExecInitQual(node->indexfiltersorig, (PlanState *) indexstate);
+ indexstate->indexfilter =
+ ExecInitQual(node->indexfilter, (PlanState *) indexstate);
+ indexstate->indexfilterorig =
+ ExecInitQual(node->indexfilterorig, (PlanState *) indexstate);
+ indexstate->indexfilterqual =
+ ExecInitQual(node->indexfilterqual, (PlanState *) indexstate);
indexstate->indexorderbyorig =
ExecInitExprList(node->indexorderbyorig, (PlanState *) indexstate);
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index fef7d62dab8..0fc0585d01d 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -637,28 +637,6 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count,
/* estimate number of main-table tuples fetched */
tuples_fetched = clamp_row_est(indexSelectivity * baserel->tuples);
- /*
- * If some filters can be evaluated on the index tuple, account for that.
- * We need to scan all tuples from pages that are not all-visible, and
- * for the remaining tuples we fetch those not eliminated by the filter.
- *
- * XXX Does this need to worry about path->path.param_info?
- *
- * XXX All of this seems overly manual / ad-hoc, surely there's a place
- * where we already do this in a more elegant manner?
- */
- if (path->indexfilters != NIL)
- {
- Selectivity sel;
-
- sel = clauselist_selectivity(root, path->indexfilters, baserel->relid,
- JOIN_INNER, NULL);
-
- tuples_fetched *= (1.0 - baserel->allvisfrac) + (baserel->allvisfrac) * sel;
-
- tuples_fetched = clamp_row_est(tuples_fetched);
- }
-
/* fetch estimated page costs for tablespace containing table */
get_tablespace_page_costs(baserel->reltablespace,
&spc_random_page_cost,
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index aec05e1a087..6f6465ea7b6 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -51,9 +51,11 @@ typedef enum
/* Data structure for collecting qual clauses that match an index */
typedef struct
{
- bool nonempty; /* True if lists are not all empty */
+ bool nonempty; /* True if lists are not all empty (does not
+ * consider the indexfilters list). */
/* Lists of IndexClause nodes, one list per index column */
List *indexclauses[INDEX_MAX_KEYS];
+ List *indexfilters;
} IndexClauseSet;
/* Per-path data used within choose_bitmap_and() */
@@ -101,11 +103,9 @@ static bool eclass_already_used(EquivalenceClass *parent_ec, Relids oldrelids,
List *indexjoinclauses);
static void get_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
- List *filters, List **bitindexpaths);
+ List **bitindexpaths);
static List *build_index_paths(PlannerInfo *root, RelOptInfo *rel,
- IndexOptInfo *index,
- IndexClauseSet *clauses,
- List *filters,
+ IndexOptInfo *index, IndexClauseSet *clauses,
bool useful_predicate,
ScanTypeControl scantype,
bool *skip_nonnative_saop,
@@ -135,8 +135,7 @@ static double adjust_rowcount_for_semijoins(PlannerInfo *root,
static double approximate_joinrel_size(PlannerInfo *root, Relids relids);
static void match_restriction_clauses_to_index(PlannerInfo *root,
IndexOptInfo *index,
- IndexClauseSet *clauseset,
- List **filters);
+ IndexClauseSet *clauseset);
static void match_join_clauses_to_index(PlannerInfo *root,
RelOptInfo *rel, IndexOptInfo *index,
IndexClauseSet *clauseset,
@@ -147,13 +146,11 @@ static void match_eclass_clauses_to_index(PlannerInfo *root,
static void match_clauses_to_index(PlannerInfo *root,
List *clauses,
IndexOptInfo *index,
- IndexClauseSet *clauseset,
- List **filters);
+ IndexClauseSet *clauseset);
static void match_clause_to_index(PlannerInfo *root,
RestrictInfo *rinfo,
IndexOptInfo *index,
- IndexClauseSet *clauseset,
- List **filters);
+ IndexClauseSet *clauseset);
static IndexClause *match_clause_to_indexcol(PlannerInfo *root,
RestrictInfo *rinfo,
int indexcol,
@@ -250,7 +247,6 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
IndexClauseSet rclauseset;
IndexClauseSet jclauseset;
IndexClauseSet eclauseset;
- List *rfilters;
ListCell *lc;
/* Skip the whole mess if no indexes */
@@ -280,15 +276,14 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
* Identify the restriction clauses that can match the index.
*/
MemSet(&rclauseset, 0, sizeof(rclauseset));
- rfilters = NIL;
- match_restriction_clauses_to_index(root, index, &rclauseset, &rfilters);
+ match_restriction_clauses_to_index(root, index, &rclauseset);
/*
* Build index paths from the restriction clauses. These will be
* non-parameterized paths. Plain paths go directly to add_path(),
* bitmap paths are added to bitindexpaths to be handled below.
*/
- get_index_paths(root, rel, index, &rclauseset, rfilters,
+ get_index_paths(root, rel, index, &rclauseset,
&bitindexpaths);
/*
@@ -312,8 +307,6 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
/*
* If we found any plain or eclass join clauses, build parameterized
* index paths using them.
- *
- * XXX Maybe pass the filters too?
*/
if (jclauseset.nonempty || eclauseset.nonempty)
consider_index_join_clauses(root, rel, index,
@@ -675,7 +668,7 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
Assert(clauseset.nonempty);
/* Build index path(s) using the collected set of clauses */
- get_index_paths(root, rel, index, &clauseset, NULL, bitindexpaths);
+ get_index_paths(root, rel, index, &clauseset, bitindexpaths);
/*
* Remember we considered paths for this set of relids.
@@ -725,7 +718,7 @@ eclass_already_used(EquivalenceClass *parent_ec, Relids oldrelids,
static void
get_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
- List *filters, List **bitindexpaths)
+ List **bitindexpaths)
{
List *indexpaths;
bool skip_nonnative_saop = false;
@@ -739,7 +732,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
* paths if possible).
*/
indexpaths = build_index_paths(root, rel,
- index, clauses, filters,
+ index, clauses,
index->predOK,
ST_ANYSCAN,
&skip_nonnative_saop,
@@ -754,7 +747,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
{
indexpaths = list_concat(indexpaths,
build_index_paths(root, rel,
- index, clauses, filters,
+ index, clauses,
index->predOK,
ST_ANYSCAN,
&skip_nonnative_saop,
@@ -794,7 +787,7 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
if (skip_nonnative_saop)
{
indexpaths = build_index_paths(root, rel,
- index, clauses, filters,
+ index, clauses,
false,
ST_BITMAPSCAN,
NULL,
@@ -848,7 +841,6 @@ static List *
build_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index,
IndexClauseSet *clauses,
- List *filters,
bool useful_predicate,
ScanTypeControl scantype,
bool *skip_nonnative_saop,
@@ -966,13 +958,16 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
/*
* If we have index-only filters, combine the clauses into a simple list and
* add the relids to outer relids.
+ *
+ * XXX Shouldn't we try to order the clauses in some particular order, say by
+ * procost / selectivity? It's difficult, though.
*/
index_filters = NIL;
- if (filters)
+ if (clauses->indexfilters)
{
ListCell *lc;
- foreach(lc, filters)
+ foreach(lc, clauses->indexfilters)
{
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
@@ -1187,7 +1182,6 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
{
IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
IndexClauseSet clauseset;
- List *filters;
List *indexpaths;
bool useful_predicate;
@@ -1232,14 +1226,14 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
* Identify the restriction clauses that can match the index.
*/
MemSet(&clauseset, 0, sizeof(clauseset));
- filters = NIL;
- match_clauses_to_index(root, clauses, index, &clauseset, &filters);
+ match_clauses_to_index(root, clauses, index, &clauseset);
/*
* If no matches so far, and the index predicate isn't useful, we
* don't want it.
*
- * XXX Maybe this should check the filterset too?
+ * XXX Maybe this should check the index filters too? nonempty=true only
+ * deals with index clauses, it does not consider filters.
*/
if (!clauseset.nonempty && !useful_predicate)
continue;
@@ -1247,13 +1241,13 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
/*
* Add "other" restriction clauses to the clauseset.
*/
- match_clauses_to_index(root, other_clauses, index, &clauseset, &filters);
+ match_clauses_to_index(root, other_clauses, index, &clauseset);
/*
* Construct paths if possible.
*/
indexpaths = build_index_paths(root, rel,
- index, &clauseset, filters,
+ index, &clauseset,
useful_predicate,
ST_BITMAPSCAN,
NULL,
@@ -1900,6 +1894,14 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
/*
* check_index_filter
* Determine whether a clause can be executed directly on the index tuple.
+ *
+ * This checks the clause references only columns that can be returned by the
+ * index, same as in index-only scan.
+ *
+ * XXX It's a bit annoying we build the bitmap from index columns over and over,
+ * so if there are multiple clauses to check, we will do that multiple times.
+ * However, it's not that expensive, there usually are only few such clauses, so
+ * it doesn't seem worth worrying about.
*/
static bool
check_index_filter(RelOptInfo *rel, IndexOptInfo *index, Node *clause)
@@ -1909,7 +1911,7 @@ check_index_filter(RelOptInfo *rel, IndexOptInfo *index, Node *clause)
Bitmapset *index_canreturn_attrs = NULL;
int i;
- /* Index-only scans must be enabled */
+ /* Index-only filters must be enabled */
if (!enable_indexonlyfilter)
return false;
@@ -2121,11 +2123,10 @@ approximate_joinrel_size(PlannerInfo *root, Relids relids)
static void
match_restriction_clauses_to_index(PlannerInfo *root,
IndexOptInfo *index,
- IndexClauseSet *clauseset,
- List **filters)
+ IndexClauseSet *clauseset)
{
/* We can ignore clauses that are implied by the index predicate */
- match_clauses_to_index(root, index->indrestrictinfo, index, clauseset, filters);
+ match_clauses_to_index(root, index->indrestrictinfo, index, clauseset);
}
/*
@@ -2133,8 +2134,6 @@ match_restriction_clauses_to_index(PlannerInfo *root,
* Identify join clauses for the rel that match the index.
* Matching clauses are added to *clauseset.
* Also, add any potentially usable join OR clauses to *joinorclauses.
- *
- * FIXME Maybe this should fill the filterset too?
*/
static void
match_join_clauses_to_index(PlannerInfo *root,
@@ -2157,7 +2156,7 @@ match_join_clauses_to_index(PlannerInfo *root,
if (restriction_is_or_clause(rinfo))
*joinorclauses = lappend(*joinorclauses, rinfo);
else
- match_clause_to_index(root, rinfo, index, clauseset, NULL);
+ match_clause_to_index(root, rinfo, index, clauseset);
}
}
@@ -2165,8 +2164,6 @@ match_join_clauses_to_index(PlannerInfo *root,
* match_eclass_clauses_to_index
* Identify EquivalenceClass join clauses for the rel that match the index.
* Matching clauses are added to *clauseset.
- *
- * XXX Maybe this should fill the filterset too?
*/
static void
match_eclass_clauses_to_index(PlannerInfo *root, IndexOptInfo *index,
@@ -2197,7 +2194,7 @@ match_eclass_clauses_to_index(PlannerInfo *root, IndexOptInfo *index,
* since for non-btree indexes the EC's equality operators might not
* be in the index opclass (cf ec_member_matches_indexcol).
*/
- match_clauses_to_index(root, clauses, index, clauseset, NULL);
+ match_clauses_to_index(root, clauses, index, clauseset);
}
}
@@ -2210,8 +2207,7 @@ static void
match_clauses_to_index(PlannerInfo *root,
List *clauses,
IndexOptInfo *index,
- IndexClauseSet *clauseset,
- List **filters)
+ IndexClauseSet *clauseset)
{
ListCell *lc;
@@ -2219,7 +2215,7 @@ match_clauses_to_index(PlannerInfo *root,
{
RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
- match_clause_to_index(root, rinfo, index, clauseset, filters);
+ match_clause_to_index(root, rinfo, index, clauseset);
}
}
@@ -2244,8 +2240,7 @@ static void
match_clause_to_index(PlannerInfo *root,
RestrictInfo *rinfo,
IndexOptInfo *index,
- IndexClauseSet *clauseset,
- List **filters)
+ IndexClauseSet *clauseset)
{
int indexcol;
@@ -2295,19 +2290,15 @@ match_clause_to_index(PlannerInfo *root,
}
}
- /* if filterset is NULL, we're done */
- if (!filters)
- return;
-
/*
- * We didn't record the clause as a regular index clause, so see if
- * we can evaluate it as an index filter.
+ * If we got here, it means we didn't record the clause as a regular index
+ * clause, but maybe we could still evaluate it as an index-only filter,
+ * without fetching the heap tuple.
+ *
+ * FIXME maybe check/prevent duplicates, like above?
*/
if ((rinfo = match_filter_to_index(root, rinfo, index)) != NULL)
- {
- /* FIXME maybe check/prevent duplicates, like above? */
- *filters = lappend(*filters, rinfo);
- }
+ clauseset->indexfilters = lappend(clauseset->indexfilters, rinfo);
}
/*
@@ -2453,12 +2444,18 @@ match_filter_to_index(PlannerInfo *root,
/*
* Historically this code has coped with NULL clauses. That's probably
* not possible anymore, but we might as well continue to cope.
+ *
+ * XXX This code is new, the comment comes from match_clause_to_indexcol.
+ * Not sure if we need to handle NULL clauses here. Maybe not.
*/
if (clause == NULL)
return NULL;
/*
* Can the clause be evaluated only using the index tuple?
+ *
+ * XXX Maybe we should just do the check_index_filter here, if it's the only
+ * thing we need to do. There's no point in having yet another function.
*/
if (!check_index_filter(index->rel, index, (Node *) rinfo->clause))
return NULL;
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index cddc87d4fb8..ebdcaef8dab 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -188,7 +188,8 @@ static SampleScan *make_samplescan(List *qptlist, List *qpqual, Index scanrelid,
TableSampleClause *tsc);
static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid,
Oid indexid, List *indexqual, List *indexqualorig,
- List *indexfilters, List *indexfiltersorig,
+ List *indexfilter, List *indexfilterorig,
+ List *indexfilterqual,
List *indexorderby, List *indexorderbyorig,
List *indexorderbyops,
ScanDirection indexscandir);
@@ -3014,6 +3015,7 @@ create_indexscan_plan(PlannerInfo *root,
{
Scan *scan_plan;
List *indexclauses = best_path->indexclauses;
+ List *indexfilters = best_path->indexfilters;
List *indexorderbys = best_path->indexorderbys;
Index baserelid = best_path->path.parent->relid;
IndexOptInfo *indexinfo = best_path->indexinfo;
@@ -3026,6 +3028,7 @@ create_indexscan_plan(PlannerInfo *root,
List *fixed_indexorderbys;
List *indexorderbyops = NIL;
ListCell *l;
+ List *filterqual;
/* it should be a base rel... */
Assert(baserelid > 0);
@@ -3045,7 +3048,7 @@ create_indexscan_plan(PlannerInfo *root,
&fixed_indexquals);
/*
- * Extract the index qual expressions (stripped of RestrictInfos) from the
+ * Extract the index filter expressions (stripped of RestrictInfos) from the
* IndexClauses list, and prepare a copy with index Vars substituted for
* table Vars. (This step also does replace_nestloop_params on the
* fixed_indexquals.)
@@ -3088,6 +3091,7 @@ create_indexscan_plan(PlannerInfo *root,
* extract_nonindex_conditions() in costsize.c.
*/
qpqual = NIL;
+ filterqual = NIL;
foreach(l, scan_clauses)
{
RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
@@ -3101,14 +3105,34 @@ create_indexscan_plan(PlannerInfo *root,
false))
continue; /* provably implied by indexquals */
qpqual = lappend(qpqual, rinfo);
+
+ /*
+ * Maybe add it to the non-index quals, i.e. those that need to be
+ * evaluated on the heap tuple. But only if we decided to not evaluate
+ * it on the index directly.
+ */
+ if (list_member_ptr(indexfilters, rinfo))
+ continue;
+
+ filterqual = lappend(filterqual, rinfo);
}
/* Sort clauses into best execution order */
- qpqual = order_qual_clauses(root, qpqual);
+ qpqual = order_qual_clauses(root, qpqual); /* XXX */
/* Reduce RestrictInfo list to bare expressions; ignore pseudoconstants */
qpqual = extract_actual_clauses(qpqual, false);
+ /*
+ * Do the same cost reordering and reduction with non-index filters.
+ *
+ * XXX This is a bit strange/wrong, because it happens after we already
+ * split the clauses into index and non-index part. So if we want to look
+ * at the cost and stop pushing stuff down, this is probably too late.
+ */
+ filterqual = order_qual_clauses(root, filterqual);
+ filterqual = extract_actual_clauses(filterqual, false);
+
/*
* We have to replace any outer-relation variables with nestloop params in
* the indexqualorig, qpqual, and indexorderbyorig expressions. A bit
@@ -3126,6 +3150,8 @@ create_indexscan_plan(PlannerInfo *root,
replace_nestloop_params(root, (Node *) stripped_indexfilters);
qpqual = (List *)
replace_nestloop_params(root, (Node *) qpqual);
+ filterqual = (List *)
+ replace_nestloop_params(root, (Node *) filterqual);
indexorderbys = (List *)
replace_nestloop_params(root, (Node *) indexorderbys);
}
@@ -3202,6 +3228,7 @@ create_indexscan_plan(PlannerInfo *root,
stripped_indexquals,
fixed_indexfilters,
stripped_indexfilters,
+ filterqual, /* non-index quals */
fixed_indexorderbys,
indexorderbys,
indexorderbyops,
@@ -5595,8 +5622,9 @@ make_indexscan(List *qptlist,
Oid indexid,
List *indexqual,
List *indexqualorig,
- List *indexfilters,
- List *indexfiltersorig,
+ List *indexfilter,
+ List *indexfilterorig,
+ List *indexfilterqual,
List *indexorderby,
List *indexorderbyorig,
List *indexorderbyops,
@@ -5613,8 +5641,9 @@ make_indexscan(List *qptlist,
node->indexid = indexid;
node->indexqual = indexqual;
node->indexqualorig = indexqualorig;
- node->indexfilters = indexfilters;
- node->indexfiltersorig = indexfiltersorig;
+ node->indexfilter = indexfilter;
+ node->indexfilterorig = indexfilterorig;
+ node->indexfilterqual = indexfilterqual;
node->indexorderby = indexorderby;
node->indexorderbyorig = indexorderbyorig;
node->indexorderbyops = indexorderbyops;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 9d48de543ab..c735d996ebd 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -667,11 +667,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->indexqualorig =
fix_scan_list(root, splan->indexqualorig,
rtoffset, NUM_EXEC_QUAL(plan));
- splan->indexfilters =
- fix_scan_list(root, splan->indexfilters,
+ splan->indexfilter =
+ fix_scan_list(root, splan->indexfilter,
rtoffset, 1);
- splan->indexfiltersorig =
- fix_scan_list(root, splan->indexfiltersorig,
+ splan->indexfilterorig =
+ fix_scan_list(root, splan->indexfilterorig,
+ rtoffset, NUM_EXEC_QUAL(plan));
+ splan->indexfilterqual =
+ fix_scan_list(root, splan->indexfilterqual,
rtoffset, NUM_EXEC_QUAL(plan));
splan->indexorderby =
fix_scan_list(root, splan->indexorderby,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 6b50d3808a1..e9bb9922be1 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1571,9 +1571,27 @@ typedef struct IndexScanState
Relation iss_RelationDesc;
struct IndexScanDescData *iss_ScanDesc;
- /* index-only filters */
- ExprState *indexfilters;
- ExprState *indexfiltersorig;
+ /*
+ * index-only filters
+ *
+ * The "qual" (in PlanState) contains all the original quals, including
+ * those that might be evaluated on the index tuple only. This is needed
+ * when the optimization can't be used (pages that are not all-visible).
+ * With the optimization enabled we also split the lists into two parts,
+ * one for clauses evaluated on the index tuple, the other for clauses
+ * that need to be evaluated on the heap tuple.
+ *
+ * indexfilter/indexfilterorig contains the quals to be evaluated on the
+ * index tuple. indexfilterqual contains quals to be evaluated on the
+ * original heap tuple.
+ *
+ * XXX The indexfilterqual name is a bit strange/misleading, as it seems
+ * like "index filter quals" but it's "non-index filter quals".
+ */
+ ExprState *indexfilter; /* evaluate on index tuple */
+ ExprState *indexfilterorig;
+ ExprState *indexfilterqual; /* evaluate on heap tuple */
+
TupleTableSlot *iss_TableSlot;
Buffer iss_VMBuffer;
IndexInfo *iss_IndexInfo;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 0b0909e209f..7241d3b3490 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -425,6 +425,17 @@ typedef struct SampleScan
* by Var nodes identifying the index columns (their varno is INDEX_VAR and
* their varattno is the index column number).
*
+ * indexfilter and indexfilterorig resembles indexqual/indexqualorig, but it
+ * contains quals that are not regular index quals, but still can be evaluated
+ * on the index tuple (in a way similar to what index-only scans do). The
+ * expressions may be more complex (not simple OpExpr etc.), but the index
+ * keys are replaced by Var nodes identifying the index column etc.
+ *
+ * indexfilterqual contains quals that can't be evaluated on the index tuple
+ * (due to referencing a column not included in the index). In principle we
+ * could simply evaluate the quals from the Plan node, but that contains all
+ * quals, including those already evaluated on the index tuple.
+ *
* indexorderbyorig is similarly the original form of any ORDER BY expressions
* that are being implemented by the index, while indexorderby is modified to
* have index column Vars on the left-hand side. Here, multiple expressions
@@ -450,8 +461,9 @@ typedef struct IndexScan
Oid indexid; /* OID of index to scan */
List *indexqual; /* list of index quals (usually OpExprs) */
List *indexqualorig; /* the same in original form */
- List *indexfilters; /* quals for included columns */
- List *indexfiltersorig; /* the same in original form */
+ List *indexfilter; /* quals for index-only filters */
+ List *indexfilterorig; /* the same in original form */
+ List *indexfilterqual; /* quals for non-index-only filters */
List *indexorderby; /* list of index ORDER BY exprs */
List *indexorderbyorig; /* the same in original form */
List *indexorderbyops; /* OIDs of sort ops for ORDER BY exprs */
--
2.41.0
