fix(orca): merge colid->attno mapping in CStatistics::AppendStats to avoid extended-stats crash

src/backend/gporca/libnaucrates/include/naucrates/statistics/CStatistics.h

@@ -153,6 +153,10 @@ class CStatistics : public IStatistics, public DbgPrintMixin<CStatistics>
 									  UlongToColRefMap *colref_mapping,
 									  BOOL must_exist);
 
+	// helper method to add attno information without remapping
+	static void AddAttnoInfo(CMemoryPool *mp, UlongToIntMap *src_attno,
+							 UlongToIntMap *dest_attno);
+
 public:
 	CStatistics &operator=(CStatistics &) = delete;
 

src/backend/gporca/libnaucrates/src/statistics/CExtendedStatsProcessor.cpp

@@ -34,9 +34,31 @@ using namespace gpopt;
 static BOOL
 IsDependencyCapablePredicate(CStatsPred *child_pred GPOS_UNUSED)
 {
+	/*
+	 * TODO: EsptPoint covers all constant comparisons (Eq/G/GEq/L/LEq, see
+	 * CStatsPredUtils::StatsCmpType()), so range predicates are let into
+	 * functional-dependency estimation where they get equality semantics.
+	 * The backend analogue, dependency_is_compatible_clause(), accepts
+	 * equality-to-pseudoconstant only; this should gate on the point
+	 * predicate's comparison type being EstatscmptEq.
+	 */
 	return child_pred->GetPredStatsType() == CStatsPred::EsptPoint;
 }
 
+/*
+ * A colid -> attno mapping entry is usable for extended-statistics estimation
+ * only if it exists and refers to a user column. Unlike the backend, ORCA has
+ * to translate its column ids back to attnos and the mapping is not
+ * guaranteed to cover every column. System columns (attno <= 0) are never
+ * covered by extended statistics and cannot be represented in a CBitSet; the
+ * backend rejects them too (dependency_is_compatible_clause()).
+ */
+static BOOL
+FUsableAttno(const INT *attnum)
+{
+	return nullptr != attnum && 0 < *attnum;
+}
+
 /*
  * choose_best_statistics
  *		Look for and return statistics with the specified 'requiredkind' which
@@ -277,6 +299,14 @@ CExtendedStatsProcessor::ApplyCorrelatedStatsToScaleFactorFilterCalculation(
 		{
 			ULONG colid = child_pred->GetColId();
 			INT *attnum = colid_to_attno_mapping->Find(&colid);
+			if (!FUsableAttno(attnum))
+			{
+				/*
+				 * Skip such clauses; they fall back to the independence
+				 * assumption.
+				 */
+				continue;
+			}
 			clauses_attnums->ExchangeSet(*attnum);
 		}
 	}
@@ -353,6 +383,12 @@ CExtendedStatsProcessor::ApplyCorrelatedStatsToScaleFactorFilterCalculation(
 
 			ULONG colid = child_pred->GetColId();
 			INT *attnum = colid_to_attno_mapping->Find(&colid);
+			if (!FUsableAttno(attnum))
+			{
+				/* no usable attno for this clause; it was never collected
+				 * into clauses_attnums by the pre-processing loop above */
+				continue;
+			}
 
 			/*
 			 * Technically we could find more than one clause for a given
@@ -365,7 +401,24 @@ CExtendedStatsProcessor::ApplyCorrelatedStatsToScaleFactorFilterCalculation(
 			 */
 			if (dependency_implies_attribute(dependency, *attnum))
 			{
-				s2 = 1 / result_histograms->Find(&colid)->GetFrequency().Get();
+				const CHistogram *histogram = result_histograms->Find(&colid);
+				if (nullptr == histogram)
+				{
+					/*
+					 * No histogram to estimate the implied clause with; give
+					 * up on the dependency for this attribute. The attnum
+					 * bit must still be cleared: the outer loop terminates
+					 * only once no dependency is fully matched by the
+					 * remaining attnums, so leaving the bit set would make
+					 * find_strongest_dependency() return the same dependency
+					 * forever. The clause stays unestimated here and is later
+					 * given the default selectivity by the regular per-column
+					 * path (see MakeHistHashMapConjFilter()).
+					 */
+					clauses_attnums->ExchangeClear(*attnum);
+					continue;
+				}
+				s2 = 1 / histogram->GetFrequency().Get();
 
 				/* mark this one as done, so we don't touch it again. */
 				child_pred->SetEstimated();
@@ -428,11 +481,22 @@ CExtendedStatsProcessor::ApplyCorrelatedStatsToNDistinctCalculation(
 		ULONG colid = *(*src_grouping_cols)[ul];
 
 		INT *attnum = colid_to_attno_mapping->Find(&colid);
-		if (!attnum)
+		if (nullptr == attnum)
 		{
+			/* no colid -> attno mapping; extended stats are unusable */
 			attnums->Release();
 			return false;
 		}
+		if (0 >= *attnum)
+		{
+			/*
+			 * System column: extended statistics never cover it and CBitSet
+			 * cannot represent it. Skip just this column; it never enters
+			 * 'attnums', so it stays unmatched below and is kept for the
+			 * regular per-column ndistinct path.
+			 */
+			continue;
+		}
 		attnums->ExchangeSet(*attnum);
 	}
 
@@ -514,6 +578,14 @@ CExtendedStatsProcessor::ApplyCorrelatedStatsToNDistinctCalculation(
 			}
 		}
 
+		if (nullptr == item)
+		{
+			/* there should be an item for every attribute combination */
+			matched->Release();
+			attnums->Release();
+			return false;
+		}
+
 		/* Form the output varinfo list, keeping only unmatched ones */
 		ULongPtrArray *new_src_grouping_cols = GPOS_NEW(mp) ULongPtrArray(mp);
 		for (ULONG ul = 0; ul < src_grouping_cols->Size(); ul++)

src/backend/gporca/libnaucrates/src/statistics/CFilterStatsProcessor.cpp

@@ -375,7 +375,16 @@ CFilterStatsProcessor::MakeHistHashMapConjFilter(
 		if (CStatsPred::EsptDisj != child_pred_stats->GetPredStatsType())
 		{
 			GPOS_ASSERT(gpos::ulong_max != colid);
-			hist_before = result_histograms->Find(&colid)->CopyHistogram();
+			const CHistogram *base_histogram = result_histograms->Find(&colid);
+			if (nullptr == base_histogram)
+			{
+				// no histogram for the filter column; estimate the clause
+				// with the default selectivity instead of dereferencing null
+				scale_factors->Append(GPOS_NEW(mp) CDouble(
+					1 / CHistogram::DefaultSelectivity.Get()));
+				continue;
+			}
+			hist_before = base_histogram->CopyHistogram();
 			GPOS_ASSERT(nullptr != hist_before);
 
 			CHistogram *result_histogram = nullptr;
@@ -438,9 +447,15 @@ CFilterStatsProcessor::MakeHistHashMapConjFilter(
 			{
 				CHistogram *result_histogram =
 					disjunctive_histograms_after->Find(&colid);
-				CStatisticsUtils::AddHistogram(mp, colid, result_histogram,
-											   result_histograms,
-											   true /* fReplaceOld */);
+				// the column may have no histogram at all (see
+				// MakeHistHashMapDisjFilter()); then there is nothing to
+				// replace and the scale factor alone carries the estimate
+				if (nullptr != result_histogram)
+				{
+					CStatisticsUtils::AddHistogram(mp, colid, result_histogram,
+												   result_histograms,
+												   true /* fReplaceOld */);
+				}
 				disjunctive_histograms_after->Release();
 
 				last_scale_factor =
@@ -525,6 +540,22 @@ CFilterStatsProcessor::MakeHistHashMapDisjFilter(
 			continue;
 		}
 
+		BOOL is_pred_simple =
+			!CStatsPredUtils::IsConjOrDisjPred(child_pred_stats);
+		CHistogram *histogram = input_histograms->Find(&colid);
+		if (is_pred_simple && nullptr == histogram)
+		{
+			// no histogram for the filter column; estimate the clause with
+			// the default selectivity instead of dereferencing null. Do this
+			// before the per-column bookkeeping below, like the unsupported
+			// predicate case, so the previous column's intermediate result
+			// is not flushed twice.
+			scale_factors->Append(GPOS_NEW(mp) CDouble(
+				1 / CHistogram::DefaultSelectivity.Get()));
+
+			continue;
+		}
+
 		if (IsNewStatsColumn(colid, previous_colid))
 		{
 			scale_factors->Append(GPOS_NEW(mp)
@@ -536,11 +567,8 @@ CFilterStatsProcessor::MakeHistHashMapDisjFilter(
 			previous_histogram = nullptr;
 		}
 
-		CHistogram *histogram = input_histograms->Find(&colid);
 		CHistogram *disjunctive_child_col_histogram = nullptr;
 
-		BOOL is_pred_simple =
-			!CStatsPredUtils::IsConjOrDisjPred(child_pred_stats);
 		BOOL is_colid_present = (gpos::ulong_max != colid);
 		UlongToHistogramMap *child_histograms = nullptr;
 		CDouble child_scale_factor(1.0);
@@ -574,8 +602,40 @@ CFilterStatsProcessor::MakeHistHashMapDisjFilter(
 			if (is_colid_present)
 			{
 				// conjunction or disjunction uses only a single column
-				disjunctive_child_col_histogram =
-					child_histograms->Find(&colid)->CopyHistogram();
+				const CHistogram *child_col_histogram =
+					child_histograms->Find(&colid);
+				if (nullptr != child_col_histogram)
+				{
+					disjunctive_child_col_histogram =
+						child_col_histogram->CopyHistogram();
+				}
+				else
+				{
+					// the column has no histogram, so the child left none
+					// behind; treat the child like a multi-column predicate
+					// and only merge its histogram map and scale factor.
+					//
+					// The multi-column path below is otherwise only reached
+					// with colid == gpos::ulong_max, which makes
+					// IsNewStatsColumn() true at the top of the loop, so it
+					// assumes the pending per-column state has already been
+					// flushed. Flush it here and forget the column id too,
+					// otherwise the pending histogram would be dropped and
+					// a later child on the same column would overwrite this
+					// child's scale factor instead of accumulating it.
+					if (nullptr != previous_histogram)
+					{
+						scale_factors->Append(
+							GPOS_NEW(mp) CDouble(previous_scale_factor.Get()));
+						CStatisticsUtils::UpdateDisjStatistics(
+							mp, non_updatable_cols, input_rows, cumulative_rows,
+							previous_histogram, disjunctive_result_histograms,
+							previous_colid);
+						previous_histogram = nullptr;
+					}
+					is_colid_present = false;
+					colid = gpos::ulong_max;
+				}
 			}
 		}
 

src/backend/gporca/libnaucrates/src/statistics/CStatistics.cpp

@@ -552,6 +552,33 @@ CStatistics::AppendStats(CMemoryPool *mp, IStatistics *input_stats)
 	CStatisticsUtils::AddWidthInfo(mp, stats->m_colid_width_mapping,
 								   m_colid_width_mapping);
 	GPOS_CHECK_ABORT;
+
+	// Also merge the colid -> attno mapping so that every column that has a
+	// histogram keeps an attno entry; extended-stats estimation relies on
+	// this invariant. The mapping may be shared with other stats objects
+	// (see ScaleStats()), so merge copy-on-write instead of mutating it.
+	if (0 < stats->m_colid_to_attno_mapping->Size())
+	{
+		UlongToIntMap *attnos_new = GPOS_NEW(mp) UlongToIntMap(mp);
+		AddAttnoInfo(mp, m_colid_to_attno_mapping, attnos_new);
+		AddAttnoInfo(mp, stats->m_colid_to_attno_mapping, attnos_new);
+		m_colid_to_attno_mapping->Release();
+		m_colid_to_attno_mapping = attnos_new;
+	}
+
+	// Attnos are only meaningful relative to m_ext_stats' base relation, so
+	// merging is only valid for stats objects describing the same relation;
+	// all callers merge stats of the same memo group / expression today.
+	GPOS_ASSERT(nullptr == m_ext_stats || nullptr == stats->m_ext_stats ||
+				m_ext_stats == stats->m_ext_stats);
+
+	if (nullptr == m_ext_stats)
+	{
+		// non-owning pointer, kept alive by the MD cache (never released by
+		// ~CStatistics and never AddRef'd)
+		m_ext_stats = stats->m_ext_stats;
+	}
+	GPOS_CHECK_ABORT;
 }
 
 // copy statistics object
@@ -776,6 +803,27 @@ CStatistics::AddAttnoInfoWithRemap(CMemoryPool *mp, UlongToIntMap *src_attno,
 	}
 }
 
+// add attno information without remapping
+void
+CStatistics::AddAttnoInfo(CMemoryPool *mp, UlongToIntMap *src_attno,
+						  UlongToIntMap *dest_attno)
+{
+	UlongToIntMapIter col_attno_map_iterator(src_attno);
+	while (col_attno_map_iterator.Advance())
+	{
+		ULONG colid = *(col_attno_map_iterator.Key());
+
+		if (nullptr == dest_attno->Find(&colid))
+		{
+			const INT *attno = col_attno_map_iterator.Value();
+			INT *attno_copy = GPOS_NEW(mp) INT(*attno);
+			BOOL result GPOS_ASSERTS_ONLY =
+				dest_attno->Insert(GPOS_NEW(mp) ULONG(colid), attno_copy);
+			GPOS_ASSERT(result);
+		}
+	}
+}
+
 // return the index of the array of upper bound ndvs to which column reference belongs
 ULONG
 CStatistics::GetIndexUpperBoundNDVs(const CColRef *colref)

src/test/regress/expected/gporca.out

@@ -15050,6 +15050,32 @@ select * from cte2 inner join cte_test3 on cte2.c = cte_test3.a;
 drop table cte_test1;
 drop table cte_test2;
 drop table cte_test3;
+-- Filter estimation with extended statistics must not crash when the memo
+-- group's cached stats were extended incrementally (histograms appended for
+-- columns that the colid -> attno mapping initially lacked). The ROLLUP is
+-- expanded into a CTE whose consumers request different column sets, so the
+-- stats of the dimension table's scan group get appended with the filter
+-- column after the initial derivation.
+create table extstats_dim (d_sk int, d_filter int, d_a int, d_b int, d_c int) distributed by (d_sk);
+create table extstats_fact (f_sk int, f_val int) distributed by (f_sk);
+insert into extstats_dim select g, g % 20, g % 3, g % 4, g % 5 from generate_series(1, 100) g;
+insert into extstats_fact select g % 100 + 1, g from generate_series(1, 1000) g;
+create statistics extstats_dim_nd (ndistinct) on d_a, d_b, d_c from extstats_dim;
+analyze extstats_dim;
+analyze extstats_fact;
+set optimizer = on;
+select count(*) from (
+  select d_a, d_b, d_c, sum(f_val) s
+  from extstats_fact, extstats_dim
+  where f_sk = d_sk and d_filter between 5 and 10
+  group by rollup(d_a, d_b, d_c)) x;
+ count 
+-------
+    34
+(1 row)
+
+reset optimizer;
+drop table extstats_fact, extstats_dim;
 -- start_ignore
 DROP SCHEMA orca CASCADE;
 NOTICE:  drop cascades to 190 other objects

src/test/regress/expected/gporca_optimizer.out

@@ -15085,6 +15085,32 @@ select * from cte2 inner join cte_test3 on cte2.c = cte_test3.a;
 drop table cte_test1;
 drop table cte_test2;
 drop table cte_test3;
+-- Filter estimation with extended statistics must not crash when the memo
+-- group's cached stats were extended incrementally (histograms appended for
+-- columns that the colid -> attno mapping initially lacked). The ROLLUP is
+-- expanded into a CTE whose consumers request different column sets, so the
+-- stats of the dimension table's scan group get appended with the filter
+-- column after the initial derivation.
+create table extstats_dim (d_sk int, d_filter int, d_a int, d_b int, d_c int) distributed by (d_sk);
+create table extstats_fact (f_sk int, f_val int) distributed by (f_sk);
+insert into extstats_dim select g, g % 20, g % 3, g % 4, g % 5 from generate_series(1, 100) g;
+insert into extstats_fact select g % 100 + 1, g from generate_series(1, 1000) g;
+create statistics extstats_dim_nd (ndistinct) on d_a, d_b, d_c from extstats_dim;
+analyze extstats_dim;
+analyze extstats_fact;
+set optimizer = on;
+select count(*) from (
+  select d_a, d_b, d_c, sum(f_val) s
+  from extstats_fact, extstats_dim
+  where f_sk = d_sk and d_filter between 5 and 10
+  group by rollup(d_a, d_b, d_c)) x;
+ count 
+-------
+    34
+(1 row)
+
+reset optimizer;
+drop table extstats_fact, extstats_dim;
 -- start_ignore
 DROP SCHEMA orca CASCADE;
 NOTICE:  drop cascades to 190 other objects