Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 14/02/2016 5:11 pm, "Tom Lane" wrote: > > David Rowley writes: > > On 12/02/2016 12:01 am, "Tom Lane" wrote: > > I can't quite understand what you're seeing here. > > The second loop is iterating through the original GROUP BY list, so it > will see again any outer Vars that were excluded by t

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

David Rowley writes: > On 12/02/2016 12:01 am, "Tom Lane" wrote: >> 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

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 12/02/2016 12:01 am, "Tom Lane" wrote: > > David Rowley 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 sl

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

David Rowley 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 g

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

This patch got a good share of review, so it's fair to move it to the next commitfest. I marked it as ready-for-committer there which seems to be the right status. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 25 January 2016 at 10:17, Tom Lane wrote: > David Rowley writes: >> I've looked into why the join is not removed; since the redundant >> GROUP BY columns are removed during planning, and since the outer >> query is planned before the sub query, then when the join removal code >> checks if the

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

David Rowley writes: > I've looked into why the join is not removed; since the redundant > GROUP BY columns are removed during planning, and since the outer > query is planned before the sub query, then when the join removal code > checks if the subquery can been removed, the subquery is yet to be

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 24 January 2016 at 08:20, Tom Lane wrote: > David Rowley writes: >> On 23 January 2016 at 12:44, Tom Lane wrote: >>> * What you did to join.sql/join.out seems a bit bizarre. The existing >>> test case that you modified was meant to test something else, and >>> conflating this behavior with t

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

David Rowley writes: > On 23 January 2016 at 12:44, Tom Lane wrote: >> * What you did to join.sql/join.out seems a bit bizarre. The existing >> test case that you modified was meant to test something else, and >> conflating this behavior with the pre-existing one (and not documenting >> it) is c

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

Julien Rouhaud writes: > I wonder if in remove_useless_groupby_columns(), in the foreach loop you > could change the > + if (bms_subset_compare(pkattnos, relattnos) == BMS_SUBSET1) > + { > by something like > + if (bms_num_members(relattnos) <= bms_num_members(pkattnos)) > +

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 23/01/2016 14:51, David Rowley wrote: > On 24 January 2016 at 00:56, Julien Rouhaud wrote: >> I wonder if in remove_useless_groupby_columns(), in the foreach loop you >> could change the >> >> + if (bms_subset_compare(pkattnos, relattnos) == BMS_SUBSET1) >> + { >> >> by something li

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 24 January 2016 at 00:56, Julien Rouhaud wrote: > I wonder if in remove_useless_groupby_columns(), in the foreach loop you > could change the > > + if (bms_subset_compare(pkattnos, relattnos) == BMS_SUBSET1) > + { > > by something like > > > + if (bms_num_members(relattnos) <=

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 23/01/2016 10:44, David Rowley wrote: > On 23 January 2016 at 12:44, Tom Lane wrote: >> I spent some time looking through this but decided that it needs some work >> to be committable. >> >> The main thing I didn't like was that identify_key_vars seems to have a >> rather poorly chosen API: it

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 23 January 2016 at 12:44, Tom Lane wrote: > I spent some time looking through this but decided that it needs some work > to be committable. > > The main thing I didn't like was that identify_key_vars seems to have a > rather poorly chosen API: it mixes up identifying a rel's pkey with > sorting

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

David Rowley writes: > [ prune_group_by_clause_59f15cf_2016-01-15.patch ] I spent some time looking through this but decided that it needs some work to be committable. The main thing I didn't like was that identify_key_vars seems to have a rather poorly chosen API: it mixes up identifying a rel'

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 14/01/2016 23:05, David Rowley wrote: > On 15 January 2016 at 00:19, Julien Rouhaud > mailto:julien.rouh...@dalibo.com>> > wrote: > > > +* Technically we could look at UNIQUE indexes > too, however we'd also +* have

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 15 January 2016 at 00:19, Julien Rouhaud wrote: > > +* Technically we could look at UNIQUE indexes too, > however we'd also > +* have to ensure that each column of the unique index had > a NOT NULL > > > s/had/has/ > > > +* constraint, however si

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 14/01/2016 14:29, Geoff Winkless wrote: > On 14 January 2016 at 13:16, Julien Rouhaud wrote: >> You're absolutely right, but in this case the comment is more like a >> reminder of a bigger comment few lines before that wasn't quoted in my mail > > Fair enough, although I have two niggles with

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 14 January 2016 at 13:16, Julien Rouhaud wrote: > You're absolutely right, but in this case the comment is more like a > reminder of a bigger comment few lines before that wasn't quoted in my mail Fair enough, although I have two niggles with that: a) the second comment could become physicall

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 14/01/2016 14:04, Geoff Winkless wrote: > On 14 January 2016 at 11:19, Julien Rouhaud wrote: >> + /* don't try anything unless there's two Vars */ >> + if (varlist == NULL || list_length(varlist) < 2) >> + continue; >> >> To be perfectly correct

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 14 January 2016 at 11:19, Julien Rouhaud wrote: > + /* don't try anything unless there's two Vars */ > + if (varlist == NULL || list_length(varlist) < 2) > + continue; > > To be perfectly correct, the comment should say "at least two Vars". Apo

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 14/01/2016 01:30, David Rowley wrote: > Many thanks for the thorough review! > And thanks for the patch and fast answer! > On 12 January 2016 at 23:37, Julien Rouhaud > wrote: > > > In identify_key_vars() > > + /* Only PK constraints are of

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

Many thanks for the thorough review! On 12 January 2016 at 23:37, Julien Rouhaud wrote: > > In identify_key_vars() > > + /* Only PK constraints are of interest for now, see comment above > */ > + if (con->contype != CONSTRAINT_PRIMARY) > + continue; > > I think the comment

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 01/12/2015 12:07, David Rowley wrote: > On 1 December 2015 at 17:09, Marko Tiikkaja > wrote: > > On 2015-12-01 05:00, David Rowley wrote: > > We already allow a SELECT's target list to contain > non-aggregated columns > in a GROUP BY query in c

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 3 December 2015 at 14:28, Peter Eisentraut wrote: > On 11/30/15 11:00 PM, David Rowley wrote: > > It seems that there's no shortage of relational databases in existence > > today which don't support this. These databases would require the GROUP > > BY clause to include the p.description column

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 11/30/15 11:00 PM, David Rowley wrote: > It seems that there's no shortage of relational databases in existence > today which don't support this. These databases would require the GROUP > BY clause to include the p.description column too. Well, actually, we implemented this because other datab

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On Mon, Nov 30, 2015 at 11:00 PM, David Rowley wrote: > There are in fact also two queries in TPC-H (Q10 and Q18) which are written > to include all of the non-aggregated column in the GROUP BY list. During a > recent test I witnessed a 50% gain in performance in Q10 by removing the > unneeded col

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 1 December 2015 at 17:09, Marko Tiikkaja wrote: > On 2015-12-01 05:00, David Rowley wrote: > >> We already allow a SELECT's target list to contain non-aggregated columns >> in a GROUP BY query in cases where the non-aggregated column is >> functionally dependent on the GROUP BY clause. >> >> F

Re: [HACKERS] Removing Functionally Dependent GROUP BY Columns

On 2015-12-01 05:00, David Rowley wrote: We already allow a SELECT's target list to contain non-aggregated columns in a GROUP BY query in cases where the non-aggregated column is functionally dependent on the GROUP BY clause. For example a query such as; SELECT p.product_id,p.description, SUM(s