RE: Partial aggregates pushdown

Hi Mr.Bruce, Mr.Pyhalov, Mr.Finnerty, hackers.

Thank you for your valuable comments. I sincerely apologize for the very late reply.
Here is a response to your comments or a fix to the patch.

Tuesday, August 8, 2023 at 3:31 Bruce Momjian
> > I have modified the program except for the point "if the version of the remote server is less than PG17".
> > Instead, we have addressed the following.
> > "If check_partial_aggregate_support is true and the remote server version is older than the local server
> > version, postgres_fdw does not assume that the partial aggregate function is on the remote server unless
> > the partial aggregate function and the aggregate function match."
> > The reason for this is to maintain compatibility with any aggregate function that does not support partial
> > aggregate in one version of V1 (V1 is PG17 or higher), even if the next version supports partial aggregate.
> > For example, string_agg does not support partial aggregation in PG15, but it will support partial aggregation
> > in PG16.
>
> Just to clarify, I think you are saying:
>
> If check_partial_aggregate_support is true and the remote server
> version is older than the local server version, postgres_fdw
> checks if the partial aggregate function exists on the remote
> server during planning and only uses it if it does.
>
> I tried to phrase it in a positive way, and mentioned the plan time
> distinction. Also, I am sorry I was away for most of July and am just
> getting to this.
Thanks for your comment. In the documentation, the description of check_partial_aggregate_support is as follows
(please see postgres-fdw.sgml).
--
check_partial_aggregate_support (boolean)
Only if this option is true, during query planning, postgres_fdw connects to the remote server and check if the remote server version is older than the local server version. If so, postgres_fdw assumes that for each built-in aggregate function, the partial aggregate function is not defined on the remote server unless the partial aggregate function and the aggregate function match. The default is false.
--

Thursday, 20 July 2023 19:23 Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>:
> Fujii(dot)Yuki(at)df(dot)MitsubishiElectric(dot)co(dot)jp писал 2023-07-19 03:43:
> > Hi Mr.Pyhalov, hackers.
>
> > 3)
> > I modified the patch to safely do a partial aggregate pushdown for
> > queries which contain having clauses.
> >
>
> Hi.
> Sorry, but I don't see how it could work.
We apologize for any inconvenience caused.
Thanks to Pyhalov's and Jim's comments, I have realized that I have made a fundamental mistake regarding the pushdown of the HAVING clause and the difficulty of achieving it performing Partial aggregate pushdown.
So, I removed the codes about pushdown of the HAVING clause performing Partial aggregate pushdown.

Thursday, 20 July 2023 19:23 Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>:
> As for changes in planner.c (setGroupClausePartial()) I have several
> questions.
>
> 1) Why don't we add non_group_exprs to pathtarget->exprs when
> partial_target->exprs is not set?
>
> 2) We replace extra->partial_target->exprs with partial_target->exprs
> after processing. Why are we sure that after this tleSortGroupRef is
> correct?
Response to 1)
The code you pointed out was unnecessary. I have removed this code.
Also, the process of adding PlaceHolderVar's expr to partial_target was missing.
So I fixed this.

Response to 2)
The making procedures extra->groupClausePartial and extra->partial_target
in make_partial_grouping_target for this patch is as follows.
STEP1. From grouping_target->exprs, extract Aggref, Var and Placeholdervar that are not included in Aggref.
STEP2. setGroupClausePartial sets the copy of original groupClause to extra->groupClausePartial
and sets the copy of original partial_target to extra->partial_target.
STEP3. setGroupClausePartial adds Var and Placeholdervar in STEP1 to partial_target.
The sortgroupref of partial_target->sortgrouprefs to be added to value is set to
(the maximum value of the existing sortgroupref) + 1.
setGroupClausePartial adds data sgc of sortgroupclause type where sgc->tlesortgroupref
matches the sortgroupref to GroupClause.
STEP4. add_new_columns_to_pathtarget adds STEP1's Aggref to partial_target.

Due to STEP2, the list of tlesortgrouprefs set in extra->groupClausePartial is not duplicated.
Also, sortgrouprefs added to extra->partial_target matches with corresponding
tlesortgrouprefs added to extra->groupClausePartial.
So these tlesortgrouprefs are correct.

Sincerely yours,
Yuuki Fujii

--
Yuuki Fujii
Information Technology R&D Center Mitsubishi Electric Corporation