1 files changed, 58 insertions, 28 deletions

diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index dd3c84a91c0..463d44a68a4 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1255,6 +1255,10 @@ choose_best_statistics(List *stats, char requiredkind,
 		/*
 		 * Collect attributes and expressions in remaining (unestimated)
 		 * clauses fully covered by this statistic object.
+		 *
+		 * We know already estimated clauses have both clause_attnums and
+		 * clause_exprs set to NULL. We leave the pointers NULL if already
+		 * estimated, or we reset them to NULL after estimating the clause.
 		 */
 		for (i = 0; i < nclauses; i++)
 		{
@@ -1758,39 +1762,65 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli
 		/* record which clauses are simple (single column or expression) */
 		simple_clauses = NULL;
 
-		listidx = 0;
+		listidx = -1;
 		foreach(l, clauses)
 		{
+			/* Increment the index before we decide if to skip the clause. */
+			listidx++;
+
 			/*
-			 * If the clause is not already estimated and is compatible with
-			 * the selected statistics object (all attributes and expressions
-			 * covered), mark it as estimated and add it to the list to
-			 * estimate.
+			 * Ignore clauses from which we did not extract any attnums or
+			 * expressions (this needs to be consistent with what we do in
+			 * choose_best_statistics).
+			 *
+			 * This also eliminates already estimated clauses - both those
+			 * estimated before and during applying extended statistics.
+			 *
+			 * XXX This check is needed because both bms_is_subset and
+			 * stat_covers_expressions return true for empty attnums and
+			 * expressions.
 			 */
-			if (!bms_is_member(listidx, *estimatedclauses) &&
-				bms_is_subset(list_attnums[listidx], stat->keys) &&
-				stat_covers_expressions(stat, list_exprs[listidx], NULL))
-			{
-				/* record simple clauses (single column or expression) */
-				if ((list_attnums[listidx] == NULL &&
-					 list_length(list_exprs[listidx]) == 1) ||
-					(list_exprs[listidx] == NIL &&
-					 bms_membership(list_attnums[listidx]) == BMS_SINGLETON))
-					simple_clauses = bms_add_member(simple_clauses,
-													list_length(stat_clauses));
-
-				/* add clause to list and mark as estimated */
-				stat_clauses = lappend(stat_clauses, (Node *) lfirst(l));
-				*estimatedclauses = bms_add_member(*estimatedclauses, listidx);
-
-				bms_free(list_attnums[listidx]);
-				list_attnums[listidx] = NULL;
-
-				list_free(list_exprs[listidx]);
-				list_exprs[listidx] = NULL;
-			}
+			if (!list_attnums[listidx] && !list_exprs[listidx])
+				continue;
 
-			listidx++;
+			/*
+			 * The clause was not estimated yet, and we've extracted either
+			 * attnums of expressions from it. Ignore it if it's not fully
+			 * covered by the chosen statistics.
+			 *
+			 * We need to check both attributes and expressions, and reject
+			 * if either is not covered.
+			 */
+			if (!bms_is_subset(list_attnums[listidx], stat->keys) ||
+				!stat_covers_expressions(stat, list_exprs[listidx], NULL))
+				continue;
+
+			/*
+			 * Now we know the clause is compatible (we have either atttnums
+			 * or expressions extracted from it), and was not estimated yet.
+			 */
+
+			/* record simple clauses (single column or expression) */
+			if ((list_attnums[listidx] == NULL &&
+				 list_length(list_exprs[listidx]) == 1) ||
+				(list_exprs[listidx] == NIL &&
+				 bms_membership(list_attnums[listidx]) == BMS_SINGLETON))
+				simple_clauses = bms_add_member(simple_clauses,
+												list_length(stat_clauses));
+
+			/* add clause to list and mark it as estimated */
+			stat_clauses = lappend(stat_clauses, (Node *) lfirst(l));
+			*estimatedclauses = bms_add_member(*estimatedclauses, listidx);
+
+			/*
+			 * Reset the pointers, so that choose_best_statistics knows this
+			 * clause was estimated and does not consider it again.
+			 */
+			bms_free(list_attnums[listidx]);
+			list_attnums[listidx] = NULL;
+
+			list_free(list_exprs[listidx]);
+			list_exprs[listidx] = NULL;
 		}
 
 		if (is_or)