Re: reorder GROUP BY list

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: reorder GROUP BY list
Date: 2006-03-05 02:21:45
Message-ID: 28973.1141525305@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)com> writes:
> Per recent discussion on -hackers, we should sometimes try to reorder
> the columns of the grouping clause to avoid redundant sorts. The
> optimizer is not currently capable of doing this, so this patch
> implements a simple hack in the analysis phase (transformGroupClause):
> if any subset of the GROUP BY clause matches a prefix of the ORDER BY
> list, that prefix is moved to the front of the GROUP BY clause.

I find this patch a tad inelegant. It seems like a confusing way to
think about the problem, and it also means that the routine is
effectively matching the sort clause and group clause twice --- once
in the code you added and once in the existing code.

The idea that I had about how the revised routine should work is
basically:

1. For each ORDER BY item:
1a. Find a matching GROUP BY item.
(Break out of loop if no match.)
1b. Add it to the output list with ORDER BY item's ordering op.
1c. Remove the matched item from the GROUP BY list.
2. For each remaining GROUP BY item:
2a. Add it to output list using default ordering op.

This might require a bit of code duplication from the two "add to output
list" steps, but I think it'd be a lot clearer. You could probably
avoid most of the code duplication by making a preliminary list of
TargetEntrys (findTargetlistEntry and the UNKNOWN->TEXT hack) before
applying the above steps. Or make a subroutine.

regards, tom lane

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message David Fetter 2006-03-05 03:24:07 Re: drop if exists remainder
Previous Message Neil Conway 2006-03-05 01:44:54 reorder GROUP BY list