On Mon, 2010-12-27 at 10:36 +0900, Itagaki Takahiro wrote:
> On Mon, Dec 27, 2010 at 02:09, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> > I have a free time and I can do a review of your patch. Please, can
> > send a last version and can send a links on documentation that you
> > used?
>
> Thanks! The latest patch attached.
> I've not written documentation yet, but I used the following site:
>
> [The SQL standard]
> - http://www.wiscorp.com/sql20nn.zip
> - http://www.wiscorp.com/sqlmultisets.zip
> [secondary information]
> - http://farrago.sourceforge.net/design/CollectionTypes.html
> - http://waelchatila.com/2005/05/18/1116485743467.html
> [Implementation in Oracle Database]
> - http://download.oracle.com/docs/cd/B28359_01/server.111/b28286/conditions006.htm
> - http://download.oracle.com/docs/cd/B28359_01/server.111/b28286/operators006.htm
>
> Here are the list of functions in the patch. Note that all of the
> functions treat arrays as one-dimensional (ex. [N][M] => [N * M])
> because there is no multi-dimensional arrays/multiset support
> in the SQL standard.
I think collect() is non-identical to fusion() but the tests don't
highlight that, so I think we need more tests to highlight null
handling. You mention that SQL standard doesn't handle multi-dim arrays,
so I think we need some tests to define and check that behaviour.
Other than that, looks like a clean and useful addition.
Without any docs the only point of reference to understand the
PostgreSQL implementation is by reading the tests. IMHO docs explain a
patch and make a review possible; they aren't something that can be
written after a review. Perhaps the nag doesn't apply as much when you
supply external references, but reviewer shouldn't have to read all of
those again. Of course, no need for docs to be perfect, just enough to
understand.
-- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
