On 4/15/25 11:41, Andrei Lepikhov wrote:
> On 4/14/25 01:26, David Rowley wrote:
>> just "continue;". The variable would only be needed if there was some
>> inner loop and we couldn't use "continue".
> +1
>> * XXX Maybe we should allow searching the expressions even if we
>> * found an attribute matching the expression? That would handle
>> * trivial expressions like "(a)" but it seems fairly useless.
>> */
>> Maybe it meant "matching the Var"?
> +1
>
> I have read your modification of comments to
> estimate_multivariate_ndistinct. It suggested the idea of letting the
> GroupVarInfo struct be exported. For now, only add_unique_group_var or
> another internal selfuncs.c routine may build this list.
> I'm not sure that the GroupVarInfo abstraction is shaped ideally at the
> moment, but it seems it may perform duties similar to RestrictInfo but
> for grouping keys: store the key, cache data about this key, cache
> statistics and estimations.
> It would be the first step in letting modules use
> estimate_multivariate_ndistinct. Likewise, we can use
> statext_clauselist_selectivity and dependencies_clauselist_selectivity.
>
> What do you think about that?
>
> P.S. statext_mcv_clauselist_selectivity deserves to be exported, too.
>
Not sure we'd want to export GroupVarInfo in the current shape. It was
meant for a short-lived local state (within a single function), which
seems quite different from what RestrictInfo does.
In any case, such rework seems out of scope for PG18 - it's much more
than a fix for the bug.
regards
--
Tomas Vondra