[Bug] Handle zero filter damping in ORCA conjunctions

[Waloid24]

Related to #2005 and #2006.

What does this PR do?

optimizer_damping_factor_filter accepts zero, but CStatisticsConfig asserts that the value is strictly positive. In assertion-enabled builds, this can trigger ORCA fallback during configuration creation.

Zero damping represents full correlation between predicates, so retain the largest scale factor, corresponding to the smallest predicate selectivity.

Type of Change

• Bug fix (non-breaking change)

Test Plan

I created tables as:

SET optimizer = off;

CREATE TABLE damping_inner (
    id integer,
    a  integer,
    b  integer,
    c  integer,
    z  integer
) USING heap DISTRIBUTED RANDOMLY;

INSERT INTO damping_inner
SELECT
    n,
    n % 10,
    (n / 10) % 5,
    (n / 50) % 4,
    0
FROM generate_series(0, 9999) AS g(n);

CREATE TABLE damping_outer (
    b integer,
    c integer,
    z integer
) USING heap DISTRIBUTED REPLICATED;

INSERT INTO damping_outer VALUES (0, 0, 0);

CREATE INDEX damping_inner_abcz
    ON damping_inner USING bitmap (a, b, c, z);

CREATE INDEX damping_inner_bcza
    ON damping_inner USING bitmap (b, c, z, a);

ANALYZE damping_inner;
ANALYZE damping_outer;

SET optimizer = on;
SET optimizer_enable_hashjoin = off;

and run a query:

SET optimizer_damping_factor_filter = 0;

EXPLAIN (ANALYZE, TIMING OFF)
SELECT i.*
FROM damping_outer AS o
CROSS JOIN damping_inner AS i
WHERE i.a = 1
  AND i.b = o.b
  AND i.c = o.c;

In explain I see Optimizer: GPORCA instead Optimizer: Postgres query optimizer as it was before.

Impact

Allows optimizer_damping_factor_filter = 0 in debug builds and makes zero-damping semantics explicit for conjunctions.
Performance:
No.

User-facing changes:
There is no fallback to Postgres optimizer when optimizer_damping_factor_filter = 0.

Dependencies:
No.

Checklist

• Followed contribution guide
• Added/updated documentation
• Reviewed code for security implications
• This PR contains AI-assisted code generation
• Requested review from cloudberry committers

---

[yjhjstz]

Thanks for the fix. I verified it on a clean --enable-cassert build of current main (1d53d08) + this patch, on a 3-segment demo cluster, using the tables/queries from the test plan.

What I confirmed

• Before the patch: SET optimizer_damping_factor_filter = 0 -> GPORCA failed to produce a plan ... DETAIL: CStatisticsConfig.cpp:44: Failed assertion: CDouble(0.0) < damping_factor_filter on every query (the test-plan query, plain a=1 AND b=0 AND c=0, plain OR).
• After the patch: the test-plan query and the plain AND/OR queries all report Optimizer: GPORCA. The assert relaxation works as advertised.

A few things I noticed while verifying (details inline):

1. The identical GUC-range/assert mismatch remains one line below for optimizer_damping_factor_groupby (GUC min is 0.0 in guc_gp.c, assert is still strict <). On the patched build SET optimizer_damping_factor_groupby = 0 still falls back with CStatisticsConfig.cpp:46: Failed assertion. Since this PR is about "zero damping triggers an assert fallback", it would be natural to fix both here.

2. The new zero-damping branch in CalcScaleFactorCumulativeConj appears to be numerically a no-op: the existing loop already yields max(MinRows, sf[0]) when the damping factor is 0, because of how CDouble clamps. I checked empirically: optimizer_damping_factor_filter = 1e-200 (which takes the old loop path) and = 0 (new branch) produce bit-identical row estimates (AND: 334, OR: 835 on the test-plan tables). So the functional change in this PR is really just the one-character assert change; the added branch could be dropped or reduced to a comment explaining why 0 degenerates to "keep the largest scale factor".

3. FYI, the test-plan query does not actually exercise the new branch's semantics: i.b = o.b AND i.c = o.c are outer-reference predicates, and CFilterStatsProcessor::SelectivityOfPredicate damps those using CStatisticsConfig::PstatsconfDefault(mp) (hard-coded 0.75), not the session config. On the patched build the query estimates rows=34 at damping 0.75 and at damping 0 -- identical. I see #2006 addresses that path, so this is just a note that #2007's "explicit zero-damping semantics for conjunctions" only applies to local predicates until #2006 lands.

4. CalcScaleFactorCumulativeDisj consumes the same GUC via DampedFilterScaleFactor but gets no explicit zero handling. With damping 0 the OR path degenerates to rows_max + (n-1) (each extra arm contributes exactly MinRows after clamping), e.g. a=1 OR b=0 OR c=0 estimates 2503 rows. If we want zero to have a stated meaning for AND, it is worth deciding what it should mean for OR too, or at least documenting that it is the clamp artifact.

5. The 0 == num_cols early return looks unreachable from the only caller (MakeHistHashMapConjFilter always appends last_scale_factor before the call), and the pre-existing loop already returned 1.0 for an empty array. It only protects the new [0] dereference; the sibling CalcScaleFactorCumulativeDisj expresses the same precondition as GPOS_ASSERT(0 < num_cols).

[yjhjstz]

Same issue one line below: optimizer_damping_factor_groupby also has GUC min 0.0 (guc_gp.c), but this assert is still strict. Verified on this patch: SET optimizer_damping_factor_groupby = 0; EXPLAIN SELECT a, b, count(*) FROM damping_inner GROUP BY a, b; still gives

INFO:  GPORCA failed to produce a plan, falling back to Postgres-based planner
DETAIL:  CStatisticsConfig.cpp:46: Failed assertion: CDouble(0.0) < damping_factor_groupby

The consumer (CStatisticsUtils::GetCumulativeNDVs -> DampedGroupByScaleFactor) already clamps with MinDistinct, so <= should be safe here as well.

[Waloid24]

You're right about optimizer_damping_factor_groupby. I wanted to send it in a separate issue, but if it's okay I'll send it here.

[yjhjstz]

I believe this branch computes exactly what the loop below already computes when the damping factor is 0, so it does not change behavior:

• CDouble clamps 0 to GPOS_FP_ABS_MIN (1e-250).
• DampedFilterScaleFactor(cfg, 1) returns 1.0, so the first term is max(MinRows, sf[0]).
• For ul >= 1, Pow(1e-250, n) re-clamps to 1e-250; sf * 1e-250 <= 1e250 * 1e-250 = 1, so max(MinRows=1.0, ...) is 1.0 for every remaining term.
• Product = max(1.0, sf[0]) -- identical to this return.

Empirically, SET optimizer_damping_factor_filter = 1e-200 (old loop path) and = 0 (this branch) give bit-identical estimates on the test-plan tables (AND: rows=334, OR: rows=835). So the comment's "rather than relying on CDouble's minimum magnitude" describes a difference that does not materialize. I would suggest dropping the branch (the assert change alone fixes the fallback) or replacing it with a comment explaining why 0 degenerates to the largest scale factor. Also note CDouble::operator== is itself a 1e-250-tolerance compare on clamped values, so the test does rely on the minimum magnitude.

[Waloid24]

Right, the comment "Handle zero explicitly rather than relying on CDouble's minimum magnitude when computing powers" is excessive.

But can we keep this branch as a quick way to return from the function to avoid the (unnecessary in this situation) loop below?

[yjhjstz]

Is this reachable? The only caller (CFilterStatsProcessor::MakeHistHashMapConjFilter) unconditionally appends last_scale_factor before calling, so Size() >= 1, and the pre-existing loop already returned 1.0 for zero iterations. It seems to exist only to guard the new (*scale_factors)[0] below. If the zero branch is kept, the sibling CalcScaleFactorCumulativeDisj expresses the same precondition as GPOS_ASSERT(0 < num_cols), which may be the more consistent form.

[Waloid24]

Right, GPOS_ASSERT(0 < num_cols) will be better for the current usage of this function. We always add one element to the scale_factors array before calling CalcScaleFactorCumulativeConj, so for the possible future changes from other contributors it will be better to express our expectations with GPOS_ASSERT.

[Waloid24]

If you look at #2006 I reuse CalcScaleFactorCumulativeConj there and it's possible that there will be no outer predicates, so in this case I expect CalcScaleFactorCumulativeConj will return 1.0. This check if (0 == num_cols) appears due to the reusage of this function in #2006.