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.
