Re: Removing Functionally Dependent GROUP BY Columns

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>, Marko Tiikkaja <marko(at)joh(dot)to>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Removing Functionally Dependent GROUP BY Columns
Date: 2016-02-11 23:01:10
Message-ID: 13858.1455231670@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> writes:
> [ prune_group_by_clause_ab4f491_2016-01-23.patch ]
> [ check_functional_grouping_refactor.patch ]

I've committed this with mostly-cosmetic revisions (principally, rewriting
a lot of the comments, which seemed on the sloppy side).

>> * Both of the loops iterating over the groupClause neglect to check
>> varlevelsup, thus leading to assert failures or worse if an outer-level
>> Var is present in the GROUP BY list. (I'm pretty sure outer Vars can
>> still be present at this point, though I might be wrong.)

> Fixed in the first loop, and the way I've rewritten the code to use
> bms_difference, there's no need to check again in the 2nd loop.

Um, AFAICS, you *do* need to check again in the second loop, else you'll
be accessing a surplusvars[] entry that might not exist at all, and in
any case might falsely tell you that you can exclude the outer var from
the new GROUP BY list.

That was the only actual bug I found, though I changed some other stuff.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kouhei Kaigai 2016-02-12 00:06:27 Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)
Previous Message Michael Paquier 2016-02-11 22:40:54 Re: Support for N synchronous standby servers - take 2