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:

Reply via email to