+ /* 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... ;)
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