Using some valuable feedback from Zhihong Yu, I fixed a flipped negation error and updated the comments.
Regards Arne ________________________________ From: Arne Roland Sent: Monday, January 17, 2022 12:25 To: Julien Rouhaud Cc: pgsql-hackers Subject: Re: missing indexes in indexlist with partitioned tables Hi, thank you for the heads up! Those files ended up accidentally in the dump from me running pg_indent. The file count was truly excessive, I should have noticed this sooner. I attached the patch without the excessive modifications. That should be way easier to read. Regards Arne
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 0ef70ad7f1..fe50919637 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -3504,7 +3504,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, Assert(list_length(exprlist) == list_length(oprlist)); /* Short-circuit if no indexes... */ - if (rel->indexlist == NIL) + if (rel->indexlist == NIL && rel->partIndexlist == NIL) return false; /* @@ -3549,7 +3549,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, return false; /* Examine each index of the relation ... */ - foreach(ic, rel->indexlist) + foreach(ic, list_concat(rel->indexlist, rel->partIndexlist)) { IndexOptInfo *ind = (IndexOptInfo *) lfirst(ic); int c; diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 337f470d58..b04b3f88ad 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -23,6 +23,7 @@ #include "postgres.h" #include "nodes/nodeFuncs.h" +#include "nodes/nodes.h" #include "optimizer/clauses.h" #include "optimizer/joininfo.h" #include "optimizer/optimizer.h" @@ -598,7 +599,7 @@ rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel) */ ListCell *lc; - foreach(lc, rel->indexlist) + foreach(lc, list_concat(rel->indexlist, rel->partIndexlist)) { IndexOptInfo *ind = (IndexOptInfo *) lfirst(lc); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 535fa041ad..5707d48dbd 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -115,8 +115,8 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, { Index varno = rel->relid; Relation relation; - bool hasindex; List *indexinfos = NIL; + List *partIndexinfos = NIL; /* * We need not lock the relation since it was already locked, either by @@ -153,17 +153,8 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, /* Retrieve the parallel_workers reloption, or -1 if not set. */ rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1); - /* - * Make list of indexes. Ignore indexes on system catalogs if told to. - * Don't bother with indexes for an inheritance parent, either. - */ - if (inhparent || - (IgnoreSystemIndexes && IsSystemRelation(relation))) - hasindex = false; - else - hasindex = relation->rd_rel->relhasindex; - - if (hasindex) + /* Make list of indexes. Ignore indexes on system catalogs if told to. */ + if (!(IgnoreSystemIndexes && IsSystemRelation(relation)) && relation->rd_rel->relhasindex) { List *indexoidlist; LOCKMODE lmode; @@ -212,10 +203,12 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, } /* - * Ignore partitioned indexes, since they are not usable for - * queries. + * Don't add partitioned indexes to the indexlist, since they are + * not usable by the executor. If they are unique add them to the + * partindexlist instead, to use for further pruning. If they + * aren't that either, simply skip them. */ - if (indexRelation->rd_rel->relkind == RELKIND_PARTITIONED_INDEX) + if (inhparent && (!index->indisunique || indexRelation->rd_rel->relkind != RELKIND_PARTITIONED_INDEX)) { index_close(indexRelation, NoLock); continue; @@ -263,7 +256,40 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, info->indexcollations[i] = indexRelation->rd_indcollation[i]; } - info->relam = indexRelation->rd_rel->relam; + /* + * Fetch the index expressions and predicate, if any. We must + * modify the copies we obtain from the relcache to have the + * correct varno for the parent relation, so that they match up + * correctly against qual clauses. + */ + info->indexprs = RelationGetIndexExpressions(indexRelation); + info->indpred = RelationGetIndexPredicate(indexRelation); + if (info->indexprs && varno != 1) + ChangeVarNodes((Node *) info->indexprs, 1, varno, 0); + if (info->indpred && varno != 1) + ChangeVarNodes((Node *) info->indpred, 1, varno, 0); + + info->unique = index->indisunique; + info->immediate = index->indimmediate; + + /* + * Don't add partitioned indexes to the indexlist, add them to the + * partindexlist instead, since they are not usable by the + * executor. + */ + if (indexRelation->rd_rel->relkind == RELKIND_PARTITIONED_INDEX) + { + index_close(indexRelation, NoLock); + partIndexinfos = lappend(partIndexinfos, info); + continue; + } + + info->hypothetical = false; + info->indrestrictinfo = NIL; /* set later, in indxpath.c */ + info->predOK = false; /* set later, in indxpath.c */ + + /* Build targetlist using the completed indexprs data */ + info->indextlist = build_index_tlist(root, info, relation); /* We copy just the fields we need, not all of rd_indam */ amroutine = indexRelation->rd_indam; @@ -283,6 +309,8 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, /* Fetch index opclass options */ info->opclassoptions = RelationGetIndexAttOptions(indexRelation, true); + info->relam = indexRelation->rd_rel->relam; + /* * Fetch the ordering information for the index, if any. */ @@ -369,28 +397,6 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, info->nulls_first = NULL; } - /* - * Fetch the index expressions and predicate, if any. We must - * modify the copies we obtain from the relcache to have the - * correct varno for the parent relation, so that they match up - * correctly against qual clauses. - */ - info->indexprs = RelationGetIndexExpressions(indexRelation); - info->indpred = RelationGetIndexPredicate(indexRelation); - if (info->indexprs && varno != 1) - ChangeVarNodes((Node *) info->indexprs, 1, varno, 0); - if (info->indpred && varno != 1) - ChangeVarNodes((Node *) info->indpred, 1, varno, 0); - - /* Build targetlist using the completed indexprs data */ - info->indextlist = build_index_tlist(root, info, relation); - - info->indrestrictinfo = NIL; /* set later, in indxpath.c */ - info->predOK = false; /* set later, in indxpath.c */ - info->unique = index->indisunique; - info->immediate = index->indimmediate; - info->hypothetical = false; - /* * Estimate the index size. If it's not a partial index, we lock * the number-of-tuples estimate to equal the parent table; if it @@ -440,6 +446,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, } rel->indexlist = indexinfos; + rel->partIndexlist = partIndexinfos; rel->statlist = get_relation_statistics(rel, relation); diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 1f33fe13c1..f4d8bd9e4a 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -334,11 +334,11 @@ struct PlannerInfo MemoryContext planner_cxt; /* context holding PlannerInfo */ - Cardinality total_table_pages; /* # of pages in all non-dummy tables of + Cardinality total_table_pages; /* # of pages in all non-dummy tables of * query */ - Selectivity tuple_fraction; /* tuple_fraction passed to query_planner */ - Cardinality limit_tuples; /* limit_tuples passed to query_planner */ + Selectivity tuple_fraction; /* tuple_fraction passed to query_planner */ + Cardinality limit_tuples; /* limit_tuples passed to query_planner */ Index qual_security_level; /* minimum security_level for quals */ /* Note: qual_security_level is zero if there are no securityQuals */ @@ -681,7 +681,7 @@ typedef struct RelOptInfo Relids relids; /* set of base relids (rangetable indexes) */ /* size estimates generated by planner */ - Cardinality rows; /* estimated number of result tuples */ + Cardinality rows; /* estimated number of result tuples */ /* per-relation planner control flags */ bool consider_startup; /* keep cheap-startup-cost paths? */ @@ -716,9 +716,10 @@ typedef struct RelOptInfo List *lateral_vars; /* LATERAL Vars and PHVs referenced by rel */ Relids lateral_referencers; /* rels that reference me laterally */ List *indexlist; /* list of IndexOptInfo */ + List *partIndexlist; /* list of IndexOptInfo */ List *statlist; /* list of StatisticExtInfo */ BlockNumber pages; /* size estimates derived from pg_class */ - Cardinality tuples; + Cardinality tuples; double allvisfrac; Bitmapset *eclass_indexes; /* Indexes in PlannerInfo's eq_classes list of * ECs that mention this rel */ @@ -841,7 +842,7 @@ struct IndexOptInfo /* index-size statistics (from pg_class and elsewhere) */ BlockNumber pages; /* number of disk pages in index */ - Cardinality tuples; /* number of index tuples in index */ + Cardinality tuples; /* number of index tuples in index */ int tree_height; /* index tree height, or -1 if unknown */ /* index descriptor information */ @@ -1139,7 +1140,7 @@ typedef struct ParamPathInfo NodeTag type; Relids ppi_req_outer; /* rels supplying parameters used by path */ - Cardinality ppi_rows; /* estimated number of result tuples */ + Cardinality ppi_rows; /* estimated number of result tuples */ List *ppi_clauses; /* join clauses available from outer rels */ } ParamPathInfo; @@ -1189,7 +1190,7 @@ typedef struct Path int parallel_workers; /* desired # of workers; 0 = not parallel */ /* estimated size/costs for path (see costsize.c for more info) */ - Cardinality rows; /* estimated number of result tuples */ + Cardinality rows; /* estimated number of result tuples */ Cost startup_cost; /* cost expended before fetching any tuples */ Cost total_cost; /* total cost (assuming all tuples fetched) */ @@ -1452,7 +1453,7 @@ typedef struct AppendPath List *subpaths; /* list of component Paths */ /* Index of first partial path in subpaths; list_length(subpaths) if none */ int first_partial_path; - Cardinality limit_tuples; /* hard limit on output tuples, or -1 */ + Cardinality limit_tuples; /* hard limit on output tuples, or -1 */ } AppendPath; #define IS_DUMMY_APPEND(p) \ @@ -1474,7 +1475,7 @@ typedef struct MergeAppendPath { Path path; List *subpaths; /* list of component Paths */ - Cardinality limit_tuples; /* hard limit on output tuples, or -1 */ + Cardinality limit_tuples; /* hard limit on output tuples, or -1 */ } MergeAppendPath; /* @@ -1772,7 +1773,7 @@ typedef struct AggPath Path *subpath; /* path representing input source */ AggStrategy aggstrategy; /* basic strategy, see nodes.h */ AggSplit aggsplit; /* agg-splitting mode, see nodes.h */ - Cardinality numGroups; /* estimated number of groups in input */ + Cardinality numGroups; /* estimated number of groups in input */ uint64 transitionSpace; /* for pass-by-ref transition data */ List *groupClause; /* a list of SortGroupClause's */ List *qual; /* quals (HAVING quals), if any */ @@ -1786,7 +1787,7 @@ typedef struct GroupingSetData { NodeTag type; List *set; /* grouping set as list of sortgrouprefs */ - Cardinality numGroups; /* est. number of result groups */ + Cardinality numGroups; /* est. number of result groups */ } GroupingSetData; typedef struct RollupData @@ -1795,7 +1796,7 @@ typedef struct RollupData List *groupClause; /* applicable subset of parse->groupClause */ List *gsets; /* lists of integer indexes into groupClause */ List *gsets_data; /* list of GroupingSetData */ - Cardinality numGroups; /* est. number of result groups */ + Cardinality numGroups; /* est. number of result groups */ bool hashable; /* can be hashed */ bool is_hashed; /* to be implemented as a hashagg */ } RollupData; @@ -1846,7 +1847,7 @@ typedef struct SetOpPath List *distinctList; /* SortGroupClauses identifying target cols */ AttrNumber flagColIdx; /* where is the flag column, if any */ int firstFlag; /* flag value for first input relation */ - Cardinality numGroups; /* estimated number of groups in input */ + Cardinality numGroups; /* estimated number of groups in input */ } SetOpPath; /* @@ -1859,7 +1860,7 @@ typedef struct RecursiveUnionPath Path *rightpath; List *distinctList; /* SortGroupClauses identifying target cols */ int wtParam; /* ID of Param representing work table */ - Cardinality numGroups; /* estimated number of groups in input */ + Cardinality numGroups; /* estimated number of groups in input */ } RecursiveUnionPath; /* @@ -2615,7 +2616,7 @@ typedef struct typedef struct { bool limit_needed; - Cardinality limit_tuples; + Cardinality limit_tuples; int64 count_est; int64 offset_est; } FinalPathExtraData; @@ -2646,15 +2647,15 @@ typedef struct JoinCostWorkspace Cost inner_rescan_run_cost; /* private for cost_mergejoin code */ - Cardinality outer_rows; - Cardinality inner_rows; - Cardinality outer_skip_rows; - Cardinality inner_skip_rows; + Cardinality outer_rows; + Cardinality inner_rows; + Cardinality outer_skip_rows; + Cardinality inner_skip_rows; /* private for cost_hashjoin code */ int numbuckets; int numbatches; - Cardinality inner_rows_total; + Cardinality inner_rows_total; } JoinCostWorkspace; /* diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out index bb5b7c47a4..1cc7f808cd 100644 --- a/src/test/regress/expected/partition_join.out +++ b/src/test/regress/expected/partition_join.out @@ -459,6 +459,19 @@ SELECT t1.a, ss.t2a, ss.t2c FROM prt1 t1 LEFT JOIN LATERAL 550 | | (12 rows) +-- join pruning +CREATE UNIQUE INDEX prt1_p1_a_idx ON prt1 (a); +EXPLAIN (COSTS OFF) +SELECT t1.a FROM prt1 t1 LEFT JOIN prt1 USING (a); + QUERY PLAN +-------------------------------- + Append + -> Seq Scan on prt1_p1 t1_1 + -> Seq Scan on prt1_p2 t1_2 + -> Seq Scan on prt1_p3 t1_3 +(4 rows) + +DROP INDEX prt1_p1_a_idx; -- bug with inadequate sort key representation SET enable_partitionwise_aggregate TO true; SET enable_hashjoin TO false; diff --git a/src/test/regress/sql/partition_join.sql b/src/test/regress/sql/partition_join.sql index 67f506361f..5f70d5d851 100644 --- a/src/test/regress/sql/partition_join.sql +++ b/src/test/regress/sql/partition_join.sql @@ -90,6 +90,13 @@ SELECT t1.a, ss.t2a, ss.t2c FROM prt1 t1 LEFT JOIN LATERAL SELECT t1.a, ss.t2a, ss.t2c FROM prt1 t1 LEFT JOIN LATERAL (SELECT t2.a AS t2a, t3.a AS t3a, t2.b t2b, t2.c t2c, least(t1.a,t2.a,t3.a) FROM prt1 t2 JOIN prt2 t3 ON (t2.a = t3.b)) ss ON t1.c = ss.t2c WHERE (t1.b + coalesce(ss.t2b, 0)) = 0 ORDER BY t1.a; +-- join pruning +CREATE UNIQUE INDEX prt1_p1_a_idx ON prt1 (a); + +EXPLAIN (COSTS OFF) +SELECT t1.a FROM prt1 t1 LEFT JOIN prt1 USING (a); + +DROP INDEX prt1_p1_a_idx; -- bug with inadequate sort key representation SET enable_partitionwise_aggregate TO true;