Re: Better solution to final adjustment of combining Aggrefs

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: Better solution to final adjustment of combining Aggrefs
Date: 2016-06-25 22:12:12
Message-ID: 20095.1466892732@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> The patch is not quite finished: as noted in the XXX comment, it'd be
> a good idea to refactor apply_partialaggref_adjustment so that it can
> share code with this function, to ensure they produce identical
> representations of the lower partial Aggref. But that will just make
> the patch longer, not any more interesting, so I left it out for now.

Here's an extended version that includes that refactoring. I ended up
deciding that apply_partialaggref_adjustment was useless: stripped of the
actual Aggref-modification logic, it's little more than an iteration over
a pathtarget list, and the fact that examining the top-level nodes is
sufficient seems like an artifact of its one existing caller rather than
general-purpose functionality. It also had no business being in tlist.c
AFAICS (the fact that putting it there required doubling the length of
tlist.c's #include list should have clued somebody that it didn't belong
there...). So I moved the loop into make_partialgroup_input_target and
created a separate function for the Aggref-munging part.

While at that I noticed that make_partialgroup_input_target was
misleadingly named and documented: what it produces is the output target
for the partial aggregation step, not the input. And for that matter
its argument is not what the rest of planner.c calls the final_target.
So this attempts to clean that up as well.

regards, tom lane

Attachment Content-Type Size
cleaner-setrefs-handling-of-partial-aggs-2.patch text/x-diff 28.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Steve Crawford 2016-06-26 00:04:46 Re: Bug in to_timestamp().
Previous Message Tom Lane 2016-06-25 17:30:03 Better solution to final adjustment of combining Aggrefs