Skip to content

[Bug] Fix ORCA selectivity damping for outer references #2006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Waloid24 wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −50
Open

[Bug] Fix ORCA selectivity damping for outer references #2006

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 22 additions & 50 deletions src/backend/gporca/libnaucrates/src/statistics/CFilterStatsProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ CFilterStatsProcessor::SelectivityOfPredicate(CMemoryPool *mp,
CColRefSet *used_col_refs = pred->DeriveUsedColumns();
CColRefSet *used_local_col_refs =
GPOS_NEW(mp) CColRefSet(mp, *used_col_refs);
ULONG num_outer_ref_preds = 0;

if (nullptr != outer_refs)
{
Expand All @@ -101,7 +100,7 @@ CFilterStatsProcessor::SelectivityOfPredicate(CMemoryPool *mp,

const COptCtxt *poctxt = COptCtxt::PoctxtFromTLS();
CMDAccessor *md_accessor = poctxt->Pmda();
// grab default stats config
// use the current optimizer statistics configuration
CStatisticsConfig *stats_config =
poctxt->GetOptimizerConfig()->GetStatsConf();
// we don't care about the width of the columns, just the row count
Expand All @@ -115,14 +114,15 @@ CFilterStatsProcessor::SelectivityOfPredicate(CMemoryPool *mp,
IStatistics *result_stats = CFilterStatsProcessor::MakeStatsFilter(
mp, dynamic_cast<CStatistics *>(base_table_stats), pred_stats, false);

CDouble result = result_stats->Rows() / base_table_stats->Rows();
BOOL have_local_preds = (result < 1.0);
const CDouble local_selectivity =
result_stats->Rows() / base_table_stats->Rows();
pred_stats->Release();
used_local_col_refs->Release();
base_table_stats->Release();
dummy_width_set->Release();

// handle outer_refs
// estimate outer predicates using statistics after local filtering
CDoubleArray *outer_scale_factors = GPOS_NEW(mp) CDoubleArray(mp);
if (nullptr != expr_with_outer_refs)
{
CExpressionArray *outer_ref_exprs =
Expand All @@ -132,7 +132,13 @@ CFilterStatsProcessor::SelectivityOfPredicate(CMemoryPool *mp,
for (ULONG ul = 0; ul < size; ul++)
{
CExpression *pexpr = (*outer_ref_exprs)[ul];
if (CUtils::FScalarConstTrue(pexpr))
{
continue;
}

CColRef *local_col_ref = nullptr;
CDouble scale_factor = 1 / CHistogram::DefaultSelectivity;

@yjhjstz yjhjstz Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) in CHistogram.cpp
  • unsupported / complex (e.g. an OR of outer refs) -> CScaleFactorUtils::DefaultJoinPredScaleFactor (100) in CJoinStatsProcessor.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.


if (CPredicateUtils::FIdentCompareOuterRefExprIgnoreCast(
pexpr, outer_refs, &local_col_ref))
Expand All @@ -144,64 +150,30 @@ CFilterStatsProcessor::SelectivityOfPredicate(CMemoryPool *mp,
GPOS_ASSERT(nullptr != local_col_ref);
CDouble ndv = result_stats->GetNDVs(local_col_ref);

if (ndv < 1.0)
{
// An NDV of less than 1 means that we have no stats on this column
result = result * CHistogram::DefaultSelectivity;
}
else
// an NDV below 1 means that we have no stats on this column
if (ndv >= 1.0)
{
result = result * (1 / ndv);
scale_factor = ndv;
}
}

@Waloid24 Waloid24 Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

?

else
{
// a comparison col op <outer ref> other than an equals
result = result * CHistogram::DefaultSelectivity;
}
num_outer_ref_preds++;
}
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
result = result * CHistogram::DefaultSelectivity;
num_outer_ref_preds++;
}
}

@Waloid24 Waloid24 Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

outer_scale_factors->Append(GPOS_NEW(mp) CDouble(scale_factor));
}

expr_with_outer_refs->Release();
outer_ref_exprs->Release();
}

// apply damping factor to the outer ref predicates whose selectivities we multiplied above
if (have_local_preds)
{
// add one for the combined non-outer refs which were dampened internally,
// but not in combination with the preds on outer refs
num_outer_ref_preds++;
}
if (1 < num_outer_ref_preds)
{
CStatisticsConfig *stats_config =
CStatisticsConfig::PstatsconfDefault(mp);

result =
std::min(result.Get() / CScaleFactorUtils::DampedFilterScaleFactor(
stats_config, num_outer_ref_preds)
.Get(),
1.0);

stats_config->Release();
}
const CDouble outer_scale_factor =
CScaleFactorUtils::CalcScaleFactorCumulativeConj(stats_config,

@yjhjstz yjhjstz Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_preds is false so no +1)
  • new: factor k (sorted desc) is divided by 0.75^k for 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.

@Waloid24 Waloid24 Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree for this case

@Waloid24 Waloid24 Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

outer_scale_factors);
outer_scale_factors->Release();
result_stats->Release();
local_expr->Release();

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;

@yjhjstz yjhjstz Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 -> old 0.0356, new 0.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=6 scores 1/(100 * max(1, 100*0.75^2)) = 1.78e-4, while a=5 AND c=<outer> scores 0.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 biases PexprBitmapSelectBestIndex toward 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.

@Waloid24 Waloid24 Sep 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Waloid24 Waloid24 Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()).

}

// create new structure from a list of statistics filters
Expand Down
Loading