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;

Reply via email to