Re: count_nulls(VARIADIC "any")

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Cc: Marko Tiikkaja <marko(at)joh(dot)to>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: count_nulls(VARIADIC "any")
Date: 2016-01-04 08:24:46
Message-ID: CAFj8pRCXeymm+xfgfCcyfx+zPypxJ1nxxcRuaAL=NetYD3v3xg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

2016-01-04 5:49 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:

> On 1/3/16 10:23 PM, Pavel Stehule wrote:
>
>> Hi
>>
>> 2016-01-03 22:49 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com
>> <mailto:Jim(dot)Nasby(at)bluetreble(dot)com>>:
>>
>> On 1/3/16 2:37 PM, Pavel Stehule wrote:
>>
>> + /* num_nulls(VARIADIC NULL) is defined as NULL */
>> + if (PG_ARGISNULL(0))
>> + return false;
>>
>>
>> Could you add to the comment explaining why that's the desired
>> behavior?
>>
>>
>> This case should be different than num_nulls(VARIADIC ARRAY[..]) - this
>> situation is really equivalent of missing data and NULL is correct
>> answer. It should not be too clean in num_nulls, but when it is cleaner
>> for num_notnulls. And more, it is consistent with other variadic
>> functions in Postgres: see concat_internal and text_format.
>>
>
> Makes sense, now that you explain it. Which is why I'm thinking it'd be
> good to add that explanation to the comment... ;)
>
>
>> Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo,
>> 0))));
>>
>>
>> Erm... is that really the way to verify that what you have is an
>> array? ISTM there should be a macro for that somewhere...
>>
>>
>> really, it is. It is used more time. Although I am not against some
>> macro, I don't think so it is necessary. The macro should not be too
>> shorter than this text.
>>
>
> Well, if there's other stuff doing that... would be nice to refactor that
> though.
>
> For brevity and example sake it'd probably be better to just use the
>> normal iterator, unless there's a serious speed difference?
>>
>>
>> The iterator does some memory allocations and some access to type cache.
>> Almost all work of iterator is useless for this case. This code is
>> developed by Marko, but I agree with this design. Using the iterator is
>> big gun for this case. I didn't any performance checks, but it should be
>> measurable for any varlena arrays.
>>
>
> Makes sense then.
>
>
+ enhanced comment
+ rewritten regress tests

Regards

Pavel

> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

Attachment Content-Type Size
num_nulls_v5.patch text/x-patch 12.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-01-04 08:32:33 Re: Some 9.5beta2 backend processes not terminating properly?
Previous Message Andres Freund 2016-01-04 08:17:05 Re: Remove Windows crash dump support?