Re: proposal: searching in array function - array_position

On 3/10/15 9:30 AM, Robert Haas wrote:
> On Sat, Mar 7, 2015 at 1:06 PM, Petr Jelinek <petr@2ndquadrant.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?

I'll take a look at it.

> 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.

Since this patch is here and ready to go I would prefer that we commit 
it and refactor later. I can tackle that unless Pavel specifically wants to.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com