From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: David Fetter <david@fetter.org>
Cc: Daniel Verite <daniel@manitou-mail.org>
Cc: PostgreSQL Development <pgsql-hackers@postgresql.org>
Cc: Vik Fearing <vik.fearing@2ndquadrant.com>
Subject: Re: popcount
Date: Mon, 18 Jan 2021 10:24:12 +0100
Message-ID: <e48355d3-efaf-e762-fffa-4b2191680292@enterprisedb.com> (raw)
In-Reply-To: <20210111161354.GA8721@fetter.org>
References: <20201230105535.GJ13234@fetter.org>
<5ea2932e-50f5-4a74-9550-5e3291b6652b@manitou-mail.org>
<20201230164108.GK13234@fetter.org>
<eb416bd3-9ee2-8a81-8cd4-d61baaf93ec2@enterprisedb.com>
<20210111161354.GA8721@fetter.org>
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.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@enterprisedb.com, david@fetter.org, daniel@manitou-mail.org, vik.fearing@2ndquadrant.com
Subject: Re: popcount
In-Reply-To: <e48355d3-efaf-e762-fffa-4b2191680292@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox