Re: Removing Functionally Dependent GROUP BY Columns

+                * 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 since NOT NULL constraints currently are don't


s/are //

>     +   /*
>     +    * If we found any surplus Vars in the GROUP BY clause, then
>     we'll build
>     +    * a new GROUP BY clause without these surplus Vars.
>     +    */
>     +   if (anysurplus)
>     +   {
>     +       List *new_groupby = NIL;
>     +
>     +       foreach (lc, root->parse->groupClause)
>     +       {
>     +           SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
>     +           TargetEntry *tle;
>     +           Var *var;
>     +
>     +           tle = get_sortgroupclause_tle(sgc, root->parse->targetList);
>     +           var = (Var *) tle->expr;
>     +
>     +           if (!IsA(var, Var))
>     +               continue;
>     + [...]
>
>     if var isn't a Var, it needs to be added in new_groupby.
>
>
> Yes you're right, well, at least I've written enough code to ensure that
> it's not needed.

Oh yes, I realize that now.

> Technically we could look inside non-Vars and check if the Expr is just
> made up of Vars from a single relation, and then we could also make that
> surplus if we find other Vars which make up the table's primary key.  I
> didn't make these changes as I thought it was a less likely scenario. It
> wouldn't be too much extra code to add however. I've went and added an
> XXX comment to explain that there might be future optimisation
> opportunities in the future around this.
>

Agreed.

> I've attached an updated patch.
>

+               /* 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".

Except the small remaining typos, this patch looks very fine to me. I
flag it as ready for committer.