Re: proposal: searching in array function - array_position

2015-03-10 16:53 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:

> On 3/10/15 9:30 AM, Robert Haas wrote:
>
>> 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?
>>
>
> 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.

I'll look on this part this evening - but I don't have any idea how to find
some common pattern yet. So I am with Jim - we can do it later.

Regards

Pavel

>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>