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

Reply via email to