On 11/1/21 22:31, Ilya Gladyshev wrote:
> Hi,
>
> On 21.10.2021 13:55, Alexander Pyhalov wrote:
>> Hi. Updated patch.
>> Now aggregates with internal states can be pushed down, if they are
>> marked as pushdown safe (this flag is set to true for min/max/sum),
>> have internal states and associated converters.
>
> I don't quite understand why this is restricted only to aggregates that
> have 'internal' state, I feel like that should be possible for any
> aggregate that has a function to convert its final result back to
> aggregate state to be pushed down. While I couldn't come up with a
> useful example for this, except maybe for an aggregate whose aggfinalfn
> is used purely for cosmetic purposes (e.g. format the result into a
> string), I still feel that it is an unnecessary restriction.
>
But it's *not* restricted to aggregates with internal state. The patch
merely requires aggregates with "internal" state to have an extra
"converter" function.
That being said, I don't think the approach used to deal with internal
state is the right one. AFAICS it simply runs the aggregate on the
remote node, finalizes is there, and then uses the converter function to
"expand" the partial result back into the internal state.
Unfortunately that only works for aggregates like "sum" where the result
is enough to rebuild the internal state, but it fails for anything more
complex (like "avg" or "var").
Earlier in this thread I mentioned this to serial/deserial functions,
and I think we need to do something like that for internal state. I.e.
we need to call the "serial" function on the remote node, and which
dumps the whole internal state, and then "deserial" on the local node.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company