Re: proposal: searching in array function - array_position

On Sat, Mar 7, 2015 at 1:06 PM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
> You still duplicate the type cache code, but many other array functions do
> that too so I will not hold that against you. (Maybe somebody should write
> separate patch that would put all that duplicate code into common function?)
>
> I think this patch is ready for committer so I am going to mark it as such.

The documentation in this patch needs some improvements to the
English. Can anyone help with that?

The documentation should discuss what happens if the array is multi-dimensional.

The code for array_offset and for array_offset_start appear to be
byte-for-byte identical. There's no comment explaining why, but I bet
it's to make the opr_sanity test pass. How about adding a comment?

The comment for array_offset_common refers to array_offset_start as
array_offset_startpos.

I am sure in agreement with the idea that it would be good to factor
out the common typecache code (for setting up my_extra). Any chance
we get a preliminary patch that does that refactoring, and then rebase
the main patch on top of it? I agree that it's not really this
patch's job to solve that problem, but it would be nice.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company