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.
