Re: Planner, check if can use consider HASH for groupings (src/backend/optimizer/plan/planner.c)

Em qui., 17 de set. de 2020 às 17:09, Tomas Vondra <
tomas(dot)vondra(at)2ndquadrant(dot)com> escreveu:

> On Thu, Sep 17, 2020 at 02:12:12PM -0300, Ranier Vilela wrote:
> >Hi,
> >
> >In case gd->any_hashable is FALSE, grouping_is_hashable is never called.
> >In this case, the planner could use HASH for groupings, but will never
> know.
> >
>
> The condition is pretty simple - if the query has grouping sets, look at
> grouping sets, otherwise look at groupClause. For this to be an issue
> the query would need to have both grouping sets and (independent) group
> clause - which AFAIK is not possible.
>
Uh?
(parse->groupClause != NIL) If different from NIL we have ((independent)
group clause), grouping_is_hashable should check?
(gd ? gd->any_hashable : grouping_is_hashable(parse->groupClause))))
If gd is not NIL and gd->any_hashtable is FALSE?

> For hashing to be worth considering, at least one grouping set has to be
> hashable - otherwise it's pointless.
>
> Granted, you could have something like
>
> GROUP BY GROUPING SETS ((a), (b)), c
>
> which I think essentially says "add c to every grouping set" and that
> will be covered by the any_hashable check.
>
I am not going into the merit of whether or not it is worth hashing.
grouping_is_hashable as a last resort, must decide.

> >Apparently gd pointer, will never be NULL there, verified with Assert(gd
> !=
> >NULL).
> >
>
> Um, what? If you add the assert right before the if condition, you won't
> even be able to do initdb. It's pretty clear it'll crash for any query
> without grouping sets.
>
Here not:
Assert(gd != NULL);
create_ordinary_grouping_paths(root, input_rel, grouped_rel,
agg_costs, gd, &extra,
&partially_grouped_rel);

Otherwise Coverity would be right.
CID 1412604 (#1 of 1): Dereference after null check (FORWARD_NULL)13.
var_deref_model: Passing null pointer gd to create_ordinary_grouping_paths,
which dereferences it. [show details
<https://scan6.coverity.com/eventId=31389494-14&modelId=31389494-0&fileInstanceId=105740687&filePath=%2Fdll%2Fpostgres%2Fsrc%2Fbackend%2Foptimizer%2Fplan%2Fplanner.c&fileStart=4053&fileEnd=4180>]

Which cannot be true, gd is never NULL here.

regards,
Ranier Vilela