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?
+ /* + * Non-null argument had better be an array. We assume that any call + * context that could let get_fn_expr_variadic return true will have + * checked that a VARIADIC-labeled parameter actually is an array. So + * it should be okay to just Assert that it's an array rather than + * doing a full-fledged error check. + */ + 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...
+ /* OK, safe to fetch the array value */ + arr = PG_GETARG_ARRAYTYPE_P(0); + + ndims = ARR_NDIM(arr); + dims = ARR_DIMS(arr); + nitems = ArrayGetNItems(ndims, dims); + + bitmap = ARR_NULLBITMAP(arr); + if (bitmap) + { + bitmask = 1; + + for (i = 0; i < nitems; i++) + { + if ((*bitmap & bitmask) == 0) + count++; + + bitmask <<= 1; + if (bitmask == 0x100) + { + bitmap++; + bitmask = 1;
For brevity and example sake it'd probably be better to just use the normal iterator, unless there's a serious speed difference?
In the unit test, I'd personally prefer just building a table with the test cases and the expected NULL/NOT NULL results, at least for all the calls that would fit that paradigm. That should significantly reduce the size of the test. Not a huge deal though...
Also, I don't think anything is testing multiples of whatever value... how 'bout change the generate_series CASE statement to >40 instead of <>40?
-- 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers