diff --git a/src/backend/gporca/libnaucrates/include/naucrates/statistics/CStatistics.h b/src/backend/gporca/libnaucrates/include/naucrates/statistics/CStatistics.h index 3dea38b58d7..50ef54a0a47 100644 --- a/src/backend/gporca/libnaucrates/include/naucrates/statistics/CStatistics.h +++ b/src/backend/gporca/libnaucrates/include/naucrates/statistics/CStatistics.h @@ -153,6 +153,10 @@ class CStatistics : public IStatistics, public DbgPrintMixin 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; diff --git a/src/backend/gporca/libnaucrates/src/statistics/CExtendedStatsProcessor.cpp b/src/backend/gporca/libnaucrates/src/statistics/CExtendedStatsProcessor.cpp index 97bcfc1ed7b..554bc06c28b 100644 --- a/src/backend/gporca/libnaucrates/src/statistics/CExtendedStatsProcessor.cpp +++ b/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++) diff --git a/src/backend/gporca/libnaucrates/src/statistics/CFilterStatsProcessor.cpp b/src/backend/gporca/libnaucrates/src/statistics/CFilterStatsProcessor.cpp index a7e458ec353..ee76fd93216 100644 --- a/src/backend/gporca/libnaucrates/src/statistics/CFilterStatsProcessor.cpp +++ b/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; diff --git a/src/backend/gporca/libnaucrates/src/statistics/CStatistics.cpp b/src/backend/gporca/libnaucrates/src/statistics/CStatistics.cpp index 80d3b289497..3f40e8a073e 100644 --- a/src/backend/gporca/libnaucrates/src/statistics/CStatistics.cpp +++ b/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) diff --git a/src/test/regress/expected/gporca.out b/src/test/regress/expected/gporca.out index f77b935cb9a..a8d2736b323 100644 --- a/src/test/regress/expected/gporca.out +++ b/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 diff --git a/src/test/regress/expected/gporca_optimizer.out b/src/test/regress/expected/gporca_optimizer.out index b8b83e01ab2..11c6111ec2d 100644 --- a/src/test/regress/expected/gporca_optimizer.out +++ b/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 diff --git a/src/test/regress/sql/gporca.sql b/src/test/regress/sql/gporca.sql index 29481d859dc..df61a968664 100644 --- a/src/test/regress/sql/gporca.sql +++ b/src/test/regress/sql/gporca.sql @@ -3762,6 +3762,28 @@ 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; +reset optimizer; +drop table extstats_fact, extstats_dim; + -- start_ignore DROP SCHEMA orca CASCADE; -- end_ignore