Conversation
SelectivityOfPredicate() applies damping to the product of local and outer predicate estimates, which can increase the result above the local selectivity. This violates the bounds of a conjunction. Outer predicate estimates use statistics after local filtering. Treat them as conditional selectivities and combine them using the existing conjunction estimator, then multiply by the local estimate. This preserves the local estimate as an upper bound and leaves it unchanged when an outer predicate has selectivity one. Use the active statistics configuration for this aggregation so that optimizer_damping_factor_filter controls the correction, rather than the hardcoded default. These estimates participate in index selection, so the change can affect planning decisions.
There was a problem hiding this comment.
Hi, @Waloid24 welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!
yjhjstz
left a comment
There was a problem hiding this comment.
Thanks for the PR. I verified it on a clean --enable-cassert build of current main (1d53d08) + this patch, on a 3-segment demo cluster, running the test-plan tables side by side against an unpatched build and capturing SelectivityOfPredicate's return value with gdb.
What I confirmed
All three numbers in the description reproduce exactly:
| query | old | new (d=0.75) | new (d=1.0) |
|---|---|---|---|
a=1 AND z=o.z |
0.17778 | 0.09999 | 0.09999 |
a=1 AND b=o.b AND c=o.c |
0.011852 | 0.0088888 | 0.0049999 |
The local estimate is now a real upper bound, and optimizer_damping_factor_filter is honored. The fix does what it says for the local-plus-outer case.
Three things I found while verifying (details inline):
-
No-local-predicate case with 3+ outer refs gets a worse estimate than before, and it flips a plan. The old code divided the whole product by
0.75^nonce; the new code goes throughCalcScaleFactorCumulativeConj, which damps the k-th sorted factor by0.75^k. For n=2 these coincide, but for n=3 the old correction is x2.37 vs new x5.6, n=4 x3.16 vs x17.7. Repro: AO table withbtree (x, y, z), NDV 3 each,WHERE i.x = o.x AND i.y = o.y AND i.z = o.z(true selectivity 0.037): old estimates 0.0878 -> Bitmap Index Scan; new estimates 0.156 -> exceeds theAO_TABLE_BTREE_INDEX_SELECTIVITY_THRESHOLD(0.10) gate inPexprBitmapSelectBestIndex, so the btree is rejected and the plan degrades to Seq Scan + Join Filter. The description's examples all include a local predicate, where the new formula is indeed better; the outer-only case moves the other way. -
Local and outer groups are now combined with pure independence -- with a single outer equality there is no damping at all (
a=1 AND b=o.b: old 0.0356 -> new 0.0200), whereas ORCA's own local path (MakeHistHashMapConjFilter) does damp two local equalities. Soa=5 AND c=<outer>is scored as more selective thana=5 AND b=6on identical NDVs, purely because of the syntactic form of the constant, which biases index ranking toward indexes covering outer-ref columns. Appending1/local_selectivityto the same array beforeCalcScaleFactorCumulativeConjwould keep the stated invariant (the product is >= its largest factor, so result <= local) while restoring damping across the two groups. -
Non-equality / complex outer predicates use
1 / DefaultSelectivity(2.5) while the real join-stats pipeline for the same predicates usesDefaultInequalityJoinPredScaleFactor(3.0) andDefaultJoinPredScaleFactor(100). Pre-existing, but this PR rewrites exactly this block withParseCmpType()already in hand, so aligning the constants would be a zero-structure change.
| stats_config->Release(); | ||
| } | ||
| const CDouble outer_scale_factor = | ||
| CScaleFactorUtils::CalcScaleFactorCumulativeConj(stats_config, |
There was a problem hiding this comment.
Reusing CalcScaleFactorCumulativeConj here changes the damping shape for the outer-only case (no local predicate), and for 3+ outer refs it is more aggressive than the code it replaces:
- old:
prod(1/ndv_i) / 0.75^n(one division for the whole group;have_local_predsis false so no +1) - new: factor k (sorted desc) is divided by
0.75^kfor k >= 1, i.e.0.75^(1+2+...+(n-1))
n=2: both x1.78. n=3: old x2.37, new x5.62. n=4: old x3.16, new x17.8.
Repro on the patched build (AO table, btree (x, y, z), NDV 3 each, 10k rows, replicated 1-row outer):
CREATE TABLE ao3 (id int, x int, y int, z int, w int) WITH (appendonly=true) DISTRIBUTED RANDOMLY;
INSERT INTO ao3 SELECT n, n % 3, (n/3) % 3, (n/9) % 3, n % 7 FROM generate_series(0, 9999) g(n);
CREATE INDEX ao3_xyz ON ao3 USING btree (x, y, z);
CREATE TABLE out3 (x int, y int, z int) DISTRIBUTED REPLICATED;
INSERT INTO out3 VALUES (0,0,0);
ANALYZE ao3; ANALYZE out3;
SET optimizer_enable_hashjoin = off;
EXPLAIN SELECT i.* FROM out3 o CROSS JOIN ao3 i WHERE i.x = o.x AND i.y = o.y AND i.z = o.z;True selectivity is 371/10000 = 0.037. gdb at the return: old 0.0878 (below the 0.10 AO-btree gate at CXformUtils.cpp AO_TABLE_BTREE_INDEX_SELECTIVITY_THRESHOLD) -> Bitmap Index Scan on ao3_xyz; new local=1 outer_sf=6.407 result=0.156 -> the btree is rejected and the plan becomes Seq Scan on ao3 + Join Filter: ((i.x = o.x) AND (i.y = o.y) AND (i.z = o.z)). So for this shape the PR both worsens the estimate and loses the index.
There was a problem hiding this comment.
I guess if the problem happens only for a few number of predicate we can keep the previous formula but cap its result at local selectivity.
| return result; | ||
| // Outer selectivities are conditional on the local filter. Damping only | ||
| // their conjunction preserves the local estimate as an upper bound. | ||
| return local_selectivity / outer_scale_factor; |
There was a problem hiding this comment.
This treats the local group and the outer group as independent (no damping between them), whereas the old code counted the local group as one more damped predicate. Two consequences I measured:
- A single outer equality with a local predicate gets no damping at all:
a=1 AND b=o.b-> old0.0356, new0.0200(= 0.1 * 1/5 exactly). - ORCA's local path (
MakeHistHashMapConjFilter->CalcScaleFactorCumulativeConj) does damp two local equalities. So with identical NDVs (say 100 each),a=5 AND b=6scores1/(100 * max(1, 100*0.75^2)) = 1.78e-4, whilea=5 AND c=<outer>scores0.01/100 = 1.0e-4. The outer-ref constant is ranked as strictly more selective than a literal purely because of its syntactic form, which biasesPexprBitmapSelectBestIndextoward indexes covering outer-ref columns.
The stated invariant (result <= local_selectivity) does not require dropping cross-group damping: if you append 1 / local_selectivity to outer_scale_factors and return 1 / CalcScaleFactorCumulativeConj(...), the product is still >= its largest factor, so the result stays <= min(local, each outer) while local and outer predicates are damped together, consistent with how purely local conjunctions are handled.
There was a problem hiding this comment.
A single outer equality with a local predicate gets no damping at all
I guess there is no problem here (CFilterStatsProcessor::SelectivityOfPredicate) because the maths tells us that damping is not needed here; we already take the statistics for outer predicates, provided after internal predicate exists, i.e. obtain NDVs after local filtering (see conditional probabilities in #2005). The purpose of damping factor is to add a degree of correlation between predicates while we've already took this into account when correcting statistics of outer predicate with an existing local predicates. So it seems like damping factor is a bit artificial here. But I agree with your comment that with this patch we will unintentionally prefer indexes covering outer-ref columns.
I'll add a patch that is coherent with your suggestion.
There was a problem hiding this comment.
When I looked closer at ORCA's local path (MakeHistHashMapConjFilter -> CalcScaleFactorCumulativeConj) I noticed that ApplyCorrelatedStatsToScaleFactorFilterCalculation function works slightly wrong (at least it seems like).
ApplyCorrelatedStatsToScaleFactorFilterCalculation function takes into account the functional dependence between predicates in the presence of extended statistics. But when do: s2 = 1 / result_histograms->Find(&colid)->GetFrequency().Get(); to process a dependent column, after the function result_histograms->Find(&colid)->GetFrequency().Get() we get always ~1.0 because we count a sum of frequencies from all buckets. But its intention seems like to get a scale factor of histogram after applying predicate. I can add a separate issue to report it there.
For example, let's say we have the same damping_partial table. And we do:
SELECT *
FROM damping_partial
WHERE a = 10 AND b = 0;Total rows = 20000
Rows with a = 10 = 1000 => P(a=10) = 0.05
Rows with b = 0 = 2000 => P(b=0) = 0.1
Dependency degree a->b = 0.5
The overall formula to count such dependencies:
/*
* Now factor in the selectivity for all the "implied" clauses into
* the final one, using this formula:
*
* P(a,b) = P(a) * (f + (1-f) * P(b))
*
* where 'f' is the degree of validity of the dependency.
*/Existing implementation uses the total frequency of the normalized b histogram, giving s2 = 1:
20000 * 0.05 * (0.5 + (1-0.5)*1) = 1000
Correct implementation (when we first get a histogram after applying a filter and then count scale factor) uses the selectivity of b = 0, giving s2 = 0.1:
20000 * 0.05 * (0.5 + (1-0.5)*0.1) = 550
The real value number of rows with a=10 and b=0 is 100.
It seems important because after the function ApplyCorrelatedStatsToScaleFactorFilterCalculation we will not consider these dependent predicates anymore (child_pred->SetEstimated()).
| } | ||
|
|
||
| CColRef *local_col_ref = nullptr; | ||
| CDouble scale_factor = 1 / CHistogram::DefaultSelectivity; |
There was a problem hiding this comment.
Minor, and pre-existing, but since this block is being rewritten with ParseCmpType() already available: these outer-ref conjuncts are join predicates from a statistics standpoint (see the comment above DeriveStatsWithOuterRefs in CJoinStatsProcessor.cpp), and the real join pipeline scores them differently:
<, <=, >, >=->CScaleFactorUtils::DefaultInequalityJoinPredScaleFactor(3.0) inCHistogram.cpp- unsupported / complex (e.g. an OR of outer refs) ->
CScaleFactorUtils::DefaultJoinPredScaleFactor(100) inCJoinStatsProcessor.cpp
Here both fall into 1 / CHistogram::DefaultSelectivity (2.5), so the same predicate is scored 2.5 for index ranking and 3.0 (or 100) for cardinality. Using the named join constants would align the two without changing the structure; 1 / CHistogram::DefaultSelectivity is also already spelled out verbatim in three other places in stats code, so a named constant would help either way.
| scale_factors->Append(GPOS_NEW(mp) CDouble(last_scale_factor)); | ||
|
|
||
| GPOS_ASSERT(nullptr != scale_factors); | ||
| CScaleFactorUtils::SortScalingFactor(scale_factors, true /* fDescending */); |
There was a problem hiding this comment.
I noticed now here we also should not sort scale_factors array because we do the same inside CalcScaleFactorCumulativeConj
| return result; | ||
| // Outer selectivities are conditional on the local filter. Damping only | ||
| // their conjunction preserves the local estimate as an upper bound. | ||
| return local_selectivity / outer_scale_factor; |
There was a problem hiding this comment.
A single outer equality with a local predicate gets no damping at all
I guess there is no problem here (CFilterStatsProcessor::SelectivityOfPredicate) because the maths tells us that damping is not needed here; we already take the statistics for outer predicates, provided after internal predicate exists, i.e. obtain NDVs after local filtering (see conditional probabilities in #2005). The purpose of damping factor is to add a degree of correlation between predicates while we've already took this into account when correcting statistics of outer predicate with an existing local predicates. So it seems like damping factor is a bit artificial here. But I agree with your comment that with this patch we will unintentionally prefer indexes covering outer-ref columns.
I'll add a patch that is coherent with your suggestion.
| return result; | ||
| // Outer selectivities are conditional on the local filter. Damping only | ||
| // their conjunction preserves the local estimate as an upper bound. | ||
| return local_selectivity / outer_scale_factor; |
There was a problem hiding this comment.
When I looked closer at ORCA's local path (MakeHistHashMapConjFilter -> CalcScaleFactorCumulativeConj) I noticed that ApplyCorrelatedStatsToScaleFactorFilterCalculation function works slightly wrong (at least it seems like).
ApplyCorrelatedStatsToScaleFactorFilterCalculation function takes into account the functional dependence between predicates in the presence of extended statistics. But when do: s2 = 1 / result_histograms->Find(&colid)->GetFrequency().Get(); to process a dependent column, after the function result_histograms->Find(&colid)->GetFrequency().Get() we get always ~1.0 because we count a sum of frequencies from all buckets. But its intention seems like to get a scale factor of histogram after applying predicate. I can add a separate issue to report it there.
For example, let's say we have the same damping_partial table. And we do:
SELECT *
FROM damping_partial
WHERE a = 10 AND b = 0;Total rows = 20000
Rows with a = 10 = 1000 => P(a=10) = 0.05
Rows with b = 0 = 2000 => P(b=0) = 0.1
Dependency degree a->b = 0.5
The overall formula to count such dependencies:
/*
* Now factor in the selectivity for all the "implied" clauses into
* the final one, using this formula:
*
* P(a,b) = P(a) * (f + (1-f) * P(b))
*
* where 'f' is the degree of validity of the dependency.
*/Existing implementation uses the total frequency of the normalized b histogram, giving s2 = 1:
20000 * 0.05 * (0.5 + (1-0.5)*1) = 1000
Correct implementation (when we first get a histogram after applying a filter and then count scale factor) uses the selectivity of b = 0, giving s2 = 0.1:
20000 * 0.05 * (0.5 + (1-0.5)*0.1) = 550
The real value number of rows with a=10 and b=0 is 100.
It seems important because after the function ApplyCorrelatedStatsToScaleFactorFilterCalculation we will not consider these dependent predicates anymore (child_pred->SetEstimated()).
| stats_config->Release(); | ||
| } | ||
| const CDouble outer_scale_factor = | ||
| CScaleFactorUtils::CalcScaleFactorCumulativeConj(stats_config, |
| result = result * (1 / ndv); | ||
| scale_factor = ndv; | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor, and pre-existing, but since this block is being rewritten with
ParseCmpType()...
So you suggest to add here a block:
else
{
scale_factor =
CScaleFactorUtils::DefaultInequalityJoinPredScaleFactor;
}?
| result = result * CHistogram::DefaultSelectivity; | ||
| num_outer_ref_preds++; | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor, and pre-existing, but since this block is being rewritten with
ParseCmpType()...
And here you suggest to add a block:
else
{
// if it is a true filter, then we had no expressions with outer refs
if (!CUtils::FScalarConstTrue(pexpr))
{
// some other expression, not of the form col op <outer ref>,
// e.g. an OR expression
scale_factor = CScaleFactorUtils::DefaultJoinPredScaleFactor;
}
}?
Could it be too severe reduction in selectivity?
| stats_config->Release(); | ||
| } | ||
| const CDouble outer_scale_factor = | ||
| CScaleFactorUtils::CalcScaleFactorCumulativeConj(stats_config, |
There was a problem hiding this comment.
I guess if the problem happens only for a few number of predicate we can keep the previous formula but cap its result at local selectivity.
Addresses the proposal #2005
What does this PR do?
SelectivityOfPredicate() applies damping to the product of local and outer predicate estimates, which can increase the result above the local selectivity. This violates the bounds of a conjunction -- adding predicates cannot produce more rows. Moreover, this change improves the optimizer’s estimate of the number of rows returned.
Outer predicate estimates use statistics after local filtering. Treat them as conditional selectivities and combine them using the existing conjunction estimator, then multiply by the local estimate. This preserves the local estimate as an upper bound and leaves it unchanged when an outer predicate has selectivity one.
Use the active statistics configuration for this aggregation so that optimizer_damping_factor_filter controls the correction, rather than the hardcoded default. These estimates participate in index selection, so the change can affect planning decisions.
Type of Change
Test Plan
Manually (with
lldb) compare selectivity estimates produced by the old and new versions of theCFilterStatsProcessor::SelectivityOfPredicatefunction.Create tables:
The generated data has the following properties:
abczAs expected (and the original version does), the query returns exactly 1,000 rows, corresponding to a selectivity of
0.1. In the debugger, the function returned0.099999003112316131; the small difference comes from ORCA's normalization of floating-point frequencies. In this case, both versions evaluate selectivity in the same way.Both
i.zand the single outer row'so.zare zero, so the additional condition excludes no rows.New version leaves the estimated selectivity as
0.099999003112316131(~0.1). While the original version returnslocal_selectivity/0.75^2 = 0.177778(far from real data distribution!) >0.1*1 = 0.1.Again, local selectivity is
0.099999003112316131(~0.1). Among1,000rows satisfyinga = 1, exactly200haveb = 0, soq_b = 0.2. Similarly,q_c = 0.25. Actual output is50rows. Atd=0.75, the patched calculation gives selectivity 0.1 * 0.2 * min(1,0.25/0.75^2) = 0.008888800276650323(or88.89rows out of10000`).The original implementation gives
0.011851733702200431or118.51rows while the actual number, again, is50rows.With
SET optimizer_damping_factor_filter = 1(predicates independence):The old version does not react to changes in the
optimizer_damping_factor_filterparameter because it always uses the default value. It returns0.011851733702200431. The new version returns0.0049999501556158062, which is consistent with the theoretical value of0.005.With
SET optimizer_damping_factor_filter = 0:Fallback to PostgreSQL optimizer.
Impact
Performance:
Yes, potentially. The planner can choose a plan which better aligns with the actual data distribution in the table.
User-facing changes:
No.
Dependencies:
No.
Checklist