On 14/01/2016 14:04, Geoff Winkless wrote: > On 14 January 2016 at 11:19, Julien Rouhaud <julien.rouh...@dalibo.com> 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". > > Apologies for butting in and I appreciate I don't have any ownership > over this codebase or right to suggest any changes, but this just > caught my eye before I could hit "delete". > > My mantra tends to be "why, not what" for inline comments; in this > case you can get the same information from the next line of code as > you get from the comment. >
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: + *[...] If there are no Vars then + * nothing need be done for this rel. If there's less than two Vars then + * there's no room to remove any, as the PRIMARY KEY must have at least one + * Var, so we can safely also do nothing if there's less than two Vars. so I assume it's ok to keep it this way. > Perhaps something like > > /* it's clearly impossible to remove duplicates if there are fewer > than two GROUPBY columns */ > > might be more helpful? > > (also sorry if I've misunderstood what it _actually_ does, I just made > an assumption based on reading this thread!) > -- Julien Rouhaud http://dalibo.com - http://dalibo.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
