Re: popcount

On 2021-01-11 17:13, David Fetter wrote:
> On Mon, Jan 11, 2021 at 03:50:54PM +0100, Peter Eisentraut wrote:
>> On 2020-12-30 17:41, David Fetter wrote:
>>>> The input may have more than 2 billion bits set to 1. The biggest possible
>>>> result should be 8 billion for bytea (1 GB with all bits set to 1).
>>>> So shouldn't this function return an int8?
>>> It does now, and thanks for looking at this.
>>
>> The documentation still reflects the previous int4 return type (in two
>> different spellings, too).
> 
> Thanks for looking this over!
> 
> Please find attached the next version with corrected documentation.

The documentation entries should be placed in alphabetical order in 
their respective tables.

For the bytea function, maybe choose a simpler example that a reader can 
compute in their head.  Also for the test cases.

In varbit.c:

The comment says

+ * Returns the number of bits set in a bit array.

but it should be "bit string".

+   int8        popcount;

should be int64.

Also, pg_popcount() returns uint64, not int64.  Perhaps overflow is not 
possible here, but perhaps a small comment about why or an assertion 
could be appropriate.

+   p = VARBITS(arg1);

Why not assign that in the declaration block as well?

This comment:

+   /*
+    * ceil(VARBITLEN(ARG1)/BITS_PER_BYTE)
+    * done to minimize branches and instructions.
+    */

I don't know what that is supposed to mean or why that kind of tuning 
would be necessary for a user-callable function.

+   popcount = pg_popcount((const char *)p, len);

The "const" is probably not necessary.

byteapopcount() looks better, but also needs int8 -> int64.