public inbox for [email protected]  
help / color / mirror / Atom feed
From: [email protected] <[email protected]>
To: Alexander Pyhalov <[email protected]>
To: Robert Haas <[email protected]>
To: Bruce Momjian <[email protected]>
To: PostgreSQL-development <[email protected]>
Cc: Finnerty, Jim <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: [email protected] <[email protected]>
Subject: RE: Partial aggregates pushdown
Date: Wed, 22 Nov 2023 10:16:16 +0000
Message-ID: <TYAPR01MB55149B8569F2FDA262654C4F95BAA@TYAPR01MB5514.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <TYTPR01MB1095486F5E3103D783AF4748995DFA@TYTPR01MB10954.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYTPR01MB10954D890D8F729E1BCE42D4F95DDA@TYTPR01MB10954.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYTPR01MB10954B4C9A791FA9D2BF692F095DCA@TYTPR01MB10954.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYTPR01MB10954B96601911FE868BF88E895DCA@TYTPR01MB10954.jpnprd01.prod.outlook.com>
	<OSZPR01MB9424BB7E885791D88B80F7B395B3A@OSZPR01MB9424.jpnprd01.prod.outlook.com>
	<OSZPR01MB94244C52ABBE67C4A033B5E495B3A@OSZPR01MB9424.jpnprd01.prod.outlook.com>
	<CA+Tgmobvja+jytj5zcEcYgqzOaeJiqrrJxgqDf1q=3k8FepuWQ@mail.gmail.com>
	<[email protected]>
	<CA+TgmoaiH_Na3y17PankZH59EgJWbgdPM-szb1eP6DQoOAMgYg@mail.gmail.com>
	<[email protected]>

Hi Mr. Haas, hackers.

Thank you for your thoughtful comments.

> From: Robert Haas <[email protected]>
> Sent: Tuesday, November 21, 2023 5:52 AM
> I do have a concern about this, though. It adds a lot of bloat. It adds a whole lot of additional entries to pg_aggregate, and
> every new aggregate we add in the future will require a bonus entry for this, and it needs a bunch of new pg_proc entries
> as well. One idea that I've had in the past is to instead introduce syntax that just does this, without requiring a separate
> aggregate definition in each case.
> For example, maybe instead of changing string_agg(whatever) to string_agg_p_text_text(whatever), you can say
> PARTIAL_AGGREGATE
> string_agg(whatever) or string_agg(PARTIAL_AGGREGATE whatever) or something. Then all aggregates could be treated
> in a generic way. I'm not completely sure that's better, but I think it's worth considering.
I believe this comment addresses a fundamental aspect of the approach.
So, firstly, could we discuss whether we should fundamentally reconsider the approach?

The approach adopted in this patch is as follows.
Approach 1: Adding partial aggregation functions to the catalogs(pg_aggregate, pg_proc)

The approach proposed by Mr.Haas is as follows.
Approach 2: Adding a keyword to the SQL syntax to indicate partial aggregation requests

The amount of code required to implement Approach 2 has not been investigated,
but comparing Approach 1 and Approach 2 in other aspects, 
I believe they each have the following advantages and disadvantages. 

1. Approach 1
(1) Advantages
(a) No need to change the SQL syntax
(2) Disadvantages
(a) Catalog bloat
As Mr.Haas pointed out, the catalog will bloat by adding partial aggregation functions (e.g. avg_p_int8(int8)) 
for each individual aggregate function (e.g. avg(int8)) in pg_aggregate and pg_proc (theoretically doubling the size).
Some PostgreSQL developers and users may find this uncomfortable.
(b) Increase in manual procedures
Developers of new aggregate functions (both built-in and user-defined) need to manually add the partial aggregation
functions when defining the aggregate functions.
However, the procedure for adding partial aggregation functions for a certain aggregate function can be automated,
so this problem can be resolved by improving the patch.
The automation method involves the core part (AggregateCreate() and related functions) that executes
the CREATE AGGREGATE command for user-defined functions.
For built-in functions, it involves generating the initial data for the pg_aggregate catalog and pg_proc catalog from pg_aggregate.dat and pg_proc.dat
(using the genbki.pl script and related scripts).

2. Approach 2
(1) Advantages
(a) No need to add partial aggregate functions to the catalogs for each aggregation
(2) Disadvantages
(a) Need to add non-standard keywords to the SQL syntax.

I did not choose Approach2 because I was not confident that the disadvantage mentioned in 2.(2)(a)
would be accepted by the PostgreSQL development community.
If it is accepted, I think Approach 2 is smarter.
Could you please provide your opinion on which
approach is preferable after comparing these two approaches?
If we cannot say anything without comparing the amount of source code, as Mr.Momjian mentioned,
we need to estimate the amount of source code required to implement Approach2.

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


view thread (16+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: RE: Partial aggregates pushdown
  In-Reply-To: <TYAPR01MB55149B8569F2FDA262654C4F95BAA@TYAPR01MB5514.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox