Re: Fix var_eq_const: sum selectivity of all matching MCV entries instead of stopping at first match

On 8/4/26 14:29, Damil Shahzad wrote:
>Tom's concern still seems important. Scanning the whole MCV list every time would cost more in the common case, and it only changes the result when the comparison operator or collation is different from the equality used to build the statistics. Before this can move forward, I think we need a stronger reason for that tradeoff.
>
>+1
>
>A cheaper way to get some benefit here without touching that tradeoff: when there are no MCV matches, var_eq_const does a second full pass over MCV list just to compute `sumcommon` - but that branch is only reached after the first loop has already scanned every entry. So `sumcommon` can be accumulated inline in that same scan, and the separate summing loop dropped. The match case is unaffected; only the no-match path gets faster, by skipping a redundant second traversal.
>
>I attached patch with these changes. What do you think?
>
>--
>Best regards,
>Ilia Evdokimov,
>Tantor Labs LLC,
>https://tantorlabs.com

Hi, Ilia, hackers

After reviewing compute_scalar_stats(), one of the functions responsible for
collecting statistics, and verifying the corresponding statistics columns in the
system catalog, I confirmed that the arrays in pg_stats for most_common_vals
and most_common_freqs always have the same number of elements.

Therefore, I agree with your suggestion and have now integrated your code into the patch.

Thanks again for your patch.

1.Attached is compute_scalar_stats(). The allocation and assignment of elements in
these two arrays are both consistently based on the variable num_mcv:
```
compute_scalar_stats()
/* Generate MCV slot entry */
if (num_mcv > 0)
{
MemoryContext old_context;
Datum *mcv_values;
float4 *mcv_freqs;

/* Must copy the target values into anl_context */
old_context = MemoryContextSwitchTo(stats->anl_context);
mcv_values = palloc_array(Datum, num_mcv);
mcv_freqs = palloc_array(float4, num_mcv);
for (i = 0; i < num_mcv; i++)
{
mcv_values[i] = datumCopy(values[track[i].first].value,
stats->attrtype->typbyval,
stats->attrtype->typlen);
mcv_freqs[i] = (double) track[i].count / (double) samplerows;
}
MemoryContextSwitchTo(old_context);

stats->stakind[slot_idx] = STATISTIC_KIND_MCV;
stats->staop[slot_idx] = mystats->eqopr;
stats->stacoll[slot_idx] = stats->attrcollid;
stats->stanumbers[slot_idx] = mcv_freqs;
stats->numnumbers[slot_idx] = num_mcv;
stats->stavalues[slot_idx] = mcv_values;
stats->numvalues[slot_idx] = num_mcv;

/*
* Accept the defaults for stats->statypid and others. They have
* been set before we were called (see vacuum.h)
*/
slot_idx++;
}

2.Verify that the `most_common_vals` and `most_common_freqs` arrays have the same length:
```SQL
analyze; --whole database

with t1 as (
select tablename,attname
,array_length(most_common_vals , 1) as length_most_common_vals
,array_length(most_common_freqs, 1) as length_most_common_freqs
from pg_catalog.pg_stats
)
select sum(1) as total
,sum(case when length_most_common_vals != length_most_common_freqs then 1 else 0 end) not_same
,sum(case when length_most_common_vals != length_most_common_freqs then 0 else 1 end) same
from t1;

database postgres return:
total | not_same | same
-------+----------+------
417 | 0 | 417
(1 row)

database xman2 return:
total | not_same | same
-------+----------+------
568 | 0 | 568
(1 row)

regards,
--
ZizhuanLiu (X-MAN) 
44973863(at)qq(dot)com