From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: David Rowley <david.rowley@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Parallel Aggregates for string_agg and array_agg
Date: Sun, 4 Mar 2018 16:54:07 +0100
Message-ID: <c302afa6-6415-27a5-336a-e1d4332fab09@2ndquadrant.com> (raw)
In-Reply-To: <CAKJS1f8cB3cc5AK=wPvByhU_jKkgd81iRt2kE6cru8uxx2HCuA@mail.gmail.com>
References: <CAKJS1f9sx_6GTcvd6TMuZnNtCh0VhBzhX6FZqw17TgVFH-ga_A@mail.gmail.com>
<20180301212631.kede4nyuttzxwoxq@alap3.anarazel.de>
<CAKJS1f-T=Ryw9OVG6hUtJ4hJoBUtM3q3bZkbCko6nU9pv+vi4A@mail.gmail.com>
<CAKJS1f8cB3cc5AK=wPvByhU_jKkgd81iRt2kE6cru8uxx2HCuA@mail.gmail.com>
Hi,
I've looked at the rebased patch you sent yesterday, and I do have a
couple of comments.
1) There seems to be forgotten declaration of initArrayResultInternal in
arrayfuncs.c. I suppose you've renamed it to initArrayResultWithSize and
moved it to a header file, and forgotten to remove this bit.
2) bytea_string_agg_deserialize has this comment:
/*
* data: technically we could reuse the buf.data buffer, but that is
* slightly larger than we need due to the extra 4 bytes for the cursor
*/
I find the argument "it has 4 extra bytes, so we'll allocate new chunk"
somewhat weak. We do allocate the memory in 2^n chunks anyway, so the
new chunk is likely to be much larger anyway. I'm not saying we need to
reuse the buffer, IMHO the savings will be non-measurable.
3) string_agg_transfn and bytea_string_agg_transfn say this"
/*
* We always append the delimiter text before the value text, even
* with the first aggregated item. The reason for this is that if
* this state needs to be combined with another state using the
* aggregate's combine function, then we need to have the delimiter
* for the first aggregated item. The first delimiter will be
* stripped off in the final function anyway. We use a little cheat
* here and store the position of the actual first value (after the
* delimiter) using the StringInfo's cursor variable. This relies on
* the cursor not being used for any other purpose.
*/
How does this make the code any simpler, compared to not adding the
delimiter (as before) and adding it when combining the values (at which
point you should know when it's actually needed)?
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, david.rowley@2ndquadrant.com, andres@anarazel.de
Subject: Re: Parallel Aggregates for string_agg and array_agg
In-Reply-To: <c302afa6-6415-27a5-336a-e1d4332fab09@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox