On Sun, Mar 1, 2026, at 01:08, Tom Lane wrote:
> Hmm ... doesn't this contradict your argument that avgfreq and
> mcv_freq need to be calculated on the same basis?  Admittedly
> that was just a heuristic, but I'm not seeing why it's wrong.

No, I didn't make that argument in my last email. But you're right that
I was wrong about that in my initial email.

Initially, I wrongly thought that the fallback mcv_freq value was a
correct calculation of the most-common-value-frequency in the
restriction. In master, without the mcv_freq fix, it's not - it's a
buggy calculation. Here's why.

The 1/rows value doesn't mean anything useful. For a unique column (no
value is more common than any other), the true MCV frequency is simply
1/tuples — a property of the data, not the query. But 1/rows varies with
the restriction: a more selective WHERE clause gives fewer rows, which
inflates the "frequency" of each unique value. That makes no sense.

Consider a unique column on a 100k-row table:

No restriction:   1/rows = 1/100000 = 0.00001  (correct)
10% selectivity:  1/rows = 1/10000  = 0.0001   (10x inflated)
1% selectivity:   1/rows = 1/1000   = 0.001    (100x inflated)
0.1% selectivity: 1/rows = 1/100    = 0.01     (1000x inflated)

The correct value is always 1/tuples = 0.00001 regardless of the
restriction.  The inflated mcv_freq then feeds into the skew ratio
(mcv_freq/avgfreq), making the planner wildly overestimate bucket skew
for selective queries.

Under the uniform restriction assumption, a base-table fraction is also
the value's fraction in the restricted output, so mcv_freq serves both
purposes without modification (with the mcv_freq fix for the fallback).

Ideally we'd have an estimate for mcv_freq in the restricted output, but
we don't. Thanks to the uniform restriction assumption, however, we can
(and should) use (the fixed) mcv_freq as is.

>> The reason for this is that estfract is calculated as:
>>     estfract = 1.0 / ndistinct;
>> where ndistinct has been adjusted to account for restriction clauses.
>> Therefore, we must also use the adjusted avgfreq when adjusting
>> estfract here:
>
> It feels like that might end up double-counting the effects of
> the restriction clauses.

Admittedly this is complex, given how many variables are involved.

Algebra to the rescue!

Code lines copy/pasted verbatim (master with just the mcv_freq fix):

mcv_freq = 1.0 / vardata.rel->tuples;
ndistinct = get_variable_numdistinct(&vardata, &isdefault);
avgfreq = (1.0 - stanullfrac) / ndistinct;
ndistinct *= vardata.rel->rows / vardata.rel->tuples;
estfract = 1.0 / ndistinct;
estfract *= *mcv_freq / avgfreq;

Helper-variables added:

mcv_freq = 1.0 / vardata.rel->tuples;
ndistinct_raw = get_variable_numdistinct(&vardata, &isdefault);
avgfreq_raw = (1.0 - stanullfrac) / ndistinct_raw;
ndistinct_adj = ndistinct_raw * (vardata.rel->rows / vardata.rel->tuples);
estfract = (1.0 / ndistinct_adj) * (mcv_freq / avgfreq_raw);

Expanding by replacing helper-variables with expressions:

estfract = (1.0 / (ndistinct_raw * (vardata.rel->rows / vardata.rel->tuples))) 
* ((1.0 / vardata.rel->tuples) / ((1.0 - stanullfrac) / ndistinct_raw));

a = estfract
b = ndistinct_raw
c = vardata.rel->rows
d = vardata.rel->tuples
e = stanullfrac

a = (1 / (b * (c / d))) * ((1 / d) / ((1 - e) / b))

Simplified using WolframAlpha:

a = 1 / ((1 - e) * c)

Plugging back the actual variables:

estfract = 1 / ((1 - stanullfrac) * vardata.rel->rows)

This makes no sense at all.

When vardata.rel->rows goes down (more selective query), estfract goes
up, i.e. the planner thinks the column is more skewed. But nothing about
the column's data changed, only the WHERE clause. A unique column has no
skew regardless of how many rows you select.

Now, let's do the same algebraic exercise, plugging in the fixed
avgfreq_adj instead:

mcv_freq = 1.0 / vardata.rel->tuples;
ndistinct_raw = get_variable_numdistinct(&vardata, &isdefault);
ndistinct_adj = ndistinct_raw * (vardata.rel->rows / vardata.rel->tuples);
avgfreq_adj = (1.0 - stanullfrac) / ndistinct_adj;
estfract = (1.0 / ndistinct_adj) * (mcv_freq / avgfreq_adj);

estfract = (1.0 / (ndistinct_raw * (vardata.rel->rows / vardata.rel->tuples))) 
* ((1.0 / vardata.rel->tuples) / ((1.0 - stanullfrac) / (ndistinct_raw * 
(vardata.rel->rows / vardata.rel->tuples))));

a = (1 / (b * (c / d))) * ((1 / d) / ((1 - e) / (b * (c / d))))

Simplified using WolframAlpha:

a = 1 / (d * (1 - e))

Notice how both ndistinct_raw (b) and rows (c) cancel — the estimate no
longer depends on the restriction selectivity.

Plugging back the variables:

estfract = 1 / (vardata.rel->tuples * (1 - stanullfrac))

Given vardata.rel->tuples != 0, this can be rewritten as:

estfract = 1 / (vardata.rel->tuples * (1 - stanullfrac))
         = (1 / vardata.rel->tuples) * (1 / (1 - stanullfrac)) -- 1/(a*b) = 
(1/a)*(1/b), a,b != 0
         = (1 / vardata.rel->tuples) / (1 - stanullfrac)       -- a * (1/b) = 
a/b, b != 0
         = mcv_freq / (1 - stanullfrac)                        -- mcv_freq = 
1.0 / vardata.rel->tuples

This makes much more sense.

Now it's just the MCV's share of non-null rows, no restriction factor.

> Anyway, we all seem to agree that s/rel->rows/rel->tuples/ is the
> correct fix for a newly-introduced bug.  I'm inclined to proceed
> by committing that fix (along with any regression test fallout)
> and then investigating the avgfreq change as an independent matter.

Yes, it seems fine to do them as separate fixes. The argument against
would be if the two separate bugs would somehow compensate for each
other, but I think they compound.

Assuming unique column, 100k rows, no nulls, 1% selectivity (rows=1000):

Master (both bugs):  estfract = 100000/1000² = 0.1      (10000x too high)
Only mcv_freq fix:   estfract = 1/1000       = 0.001    (100x too high)
Both fixes:          estfract = 1/100000     = 0.00001  (correct)

/Joel


Reply via email to