Re: Performance Enhancement/Fix for Array Utility Functions

On Wed, Mar 31, 2010 at 9:47 AM, Mike Lewis <mikelikespie@gmail.com> wrote:
> Thanks. Added it.
>
> https://commitfest.postgresql.org/action/patch_view?id=292

I have reviewed this patch; this is my review:

Regression tests pass with assertions enabled.

Performance gains reported by author confirmed.

The existence and naming of ARR_MAX_HEADER_SIZE is somewhat dubious,
as it is:

* Used in exactly one place (not necessarily a reason why it should
not be reified into a stand-alone definition, though, but
something to consider)

* The array header refers to the NULL bitmap as well, but the
interpretation used by the patch does not.

I think this patch is safe, as all the array fields required are
before the null bitmap, but I think the naming of this definition
is very misleading.

Generally I think the delimited untoasting of metadata from arrays
separately from the payload is Not A Bad Idea.

fdr