On Wed, 2011-11-16 at 17:53 -0500, Tom Lane wrote:
> I noticed that << and >> are not marked as commutator operators,
> though a naive view of their semantics suggests they should be.
> However, I realized that there might be edge cases I wasn't thinking
> about, so I went looking in the patch to try to confirm this.  And
> I found neither a single line of documentation about it, nor a single
> comment in that hairy little nest of unobvious tests that calls itself
> range_cmp_bounds.  I am of the opinion that that routine not only
> requires a comment, but very possibly a comment longer than the routine
> itself.  What's more, if it's this complicated to code, surely it would
> be a good idea for the user-facing documentation to explain exactly
> what we think before/after mean?
> In general, the level of commenting in the rangetypes code seems far short
> of what I'd consider acceptable for Postgres code.  I plan to fix some
> of that myself, but I do not wish to reverse-engineer what the heck
> range_cmp_bounds thinks it's doing.

Yikes! While commenting the code, it turns out that I missed the case
where the values match and they are both exclusive; but one is upper and
the other lower. Worse than that, there were apparently some bogus test
results that expected the wrong output. Mea culpa.

Patch attached. I'll do another pass over some of the comments in other
parts of the code.

And to your original question, it seems that << and >> should be
commutators. Perhaps I had a reason earlier, but it is escaping me now.
What edge cases did you have in mind? 

        Jeff Davis

Attachment: cmp-bounds-20111117.gz
Description: GNU Zip compressed data

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to