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)
[email protected]
v3-0001-Fix-var_eq_const-sum-selectivity-of-all-matching-.patch
Description: Binary data
