Re: proposal: searching in array function - array_position

Поиск
Список
Период
Сортировка
От Pavel Stehule
Тема Re: proposal: searching in array function - array_position
Дата
Msg-id CAFj8pRATG-0fQoC7NxDqVws8ak1R0VL7vg51JAYTn7rAMm1xPg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: proposal: searching in array function - array_position  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: proposal: searching in array function - array_position  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
Дерево обсуждения


2015-03-10 15:30 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
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?

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.

yes, it is a reason. I'll comment it.

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.

The common part is following code:

        my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
        if (my_extra == NULL)
        {
                fcinfo->flinfo->fn_extra = MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
                                                                                                          sizeof(ArrayMetaState));
                my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
                my_extra->element_type = ~element_type;
        }

and

        if (my_extra->element_type != element_type)
        {
                get_typlenbyvalalign(element_type,
                                                 &my_extra->typlen,
                                                 &my_extra->typbyval,
                                                 &my_extra->typalign);

                my_extra->element_type = element_type;
        }


so we can design function like

(ArrayMetaState *)
GetCachedArrayMetaState(FunctionCallInfo fcinfo, Oid element_type, bool *reused)
{
   ArrayMetaState *state = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
   if (state == NULL)
   {
         fcinfo->flinfo->fn_extra = MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
                                                                                                          sizeof(ArrayMetaState));
         state = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
         state->element_type = ~element_type;
   }
  if (state->element_type != element_type)
  {
         get_typlenbyvalalign(element_type,
                                                 &my_extra->typlen,
                                                 &my_extra->typbyval,
                                                 &my_extra->typalign);

          state->element_type = element_type;
          *resused = false;
  }
  else
        *reused = true;
}

Usage in code:

array_state = GetCachedArrayMetaState(fceinfo, element_type, &reused);
if (!reused)
{
    // some other initialization
}
 
The content is relatively clean, but the most big problem is naming (as usual)

Comments?

Regards

Pavel


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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Pavel Stehule
Дата:
Сообщение: Re: proposal: disallow operator "=>" and use it for named parameters
Следующее
От: Tom Lane
Дата:
Сообщение: Re: Precedence of standard comparison operators