On Mon, May 23, 2011 at 6:54 PM, Alexander Korotkov
<aekorotkov@gmail.com> wrote:
> Second version of patch attached. Main changes:
> 1) ANY and ALL keywords handling.
> 2) Collecting statistics of array length. It is used in "column <@ const".
> 3) Relatively accurate estimation of "column <@ const" selectivity. This
> estimation is most hard part of patch. I'm warrying that there could be too
> expensibe calculations for estimation. Also, likely current comments don't
> clearify anything...
Just had a brief look at this ahead of starting review.
Initial comments are that the code is well structured and I doubt
there will be problems at the code level. Looks like a good patch.
At the moment I see no tests. If this code will be exercised by
existing tests then you should put some notes with the patch to
explain that, or at least provide some pointers as to how I might test
this.
Also, I'd like to see some more explanation. Either in comments, or
just as a post to hackers. That saves me time, but we need to be clear
about what this does and does not do, what it might do in the future
etc.. 3+ years from now we need to be able to remember what the code
was supposed to do. You will forget yourself in time, if you write
enough patches. Based on this, I think you'll be writing quite a few
more.
And of course, a few lines for the docs also.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
