Re: Eager aggregation, take 3

On Sat, Oct 5, 2024 at 6:23 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>
> On Fri, Sep 27, 2024 at 11:53 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> > Here is an updated version of this patch that fixes the rowcount
> > estimate issue along this routine. (see set_joinpath_size.)
>

in the function setup_eager_aggregation,
can we be more conservative about cases where eager aggregation can be applied.
I see the following case where eager aggregation is not OK.
we can return earlier, so we won't call
create_grouping_expr_infos(root); create_agg_clause_infos(root);
, so we can avoid unintended consequences.

1. root->parse->resultRelation > 0
just be 100% sure we are only dealing with SELECT, or we can add
Assert at the end of setup_eager_aggregation.
2. join type is FULL JOIN, (i am not sure about other Semijoins and
anti-semijoins types).
3. root->parse->windowClause != NIL

I am not sure whether enable_eager_aggregate can be useful when the
LIMIT clause is there,
the code comment not mentioned.

I am also not sure about the Locking Clause, since the code is not mentioned.
EXPLAIN (COSTS OFF, settings, verbose)
SELECT avg(t2.c)
FROM (select * from eager_agg_t1 for update) t1 JOIN (select * from
eager_agg_t2 for update) t2 ON t1.b = t2.b GROUP BY t1.a;
can eager aggregate apply to above query?

in struct PlannerInfo.
/* list of AggClauseInfos */
List *agg_clause_list;
/* list of GroupExprInfos */
List *group_expr_list;
/* list of plain Vars contained in targetlist and havingQual */
List *tlist_vars;
we can comment that that agg_clause_list, tlist_vars are unique.

lack doc entry in doc/src/sgml/config.sgml
we can put after varlistentry enable_bitmapscan
we can at least mention that
enable_eager_aggregate, The default value is <literal>off</literal>.

There are no tests related to aggregate with filter clauses.
currently seems to support it.

some of the "foreach" can be rewritten to foreach_node
see
https://git.postgresql.org/cgit/postgresql.git/commit/?id=14dd0f27d7cd56ffae9ecdbe324965073d01a9ff