builtins: support bit_count for bytea and varbit

[andrew-delph]

Fixes: #114817

Release note (sql change): Added the bit_count builtin function
for BIT and BYTEA types.

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

All committers have signed the CLA.

[cockroach-teamcity]

This change is 

[rafiss]

thanks for your contribution!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andrew-delph and @michae2)

---

pkg/sql/sem/builtins/builtins.go line 243 at r1 (raw file):

	"bit_count": makeBuiltin(tree.FunctionProperties{Category: builtinconstants.CategoryString},
		stringOverload1(

could you say more about why you added a string overload? it does not appear that postgres has one. i don't understand how it's expected to work on an input like: bit_count('abc')

---

pkg/sql/sem/builtins/builtins.go line 259 at r1 (raw file):

				total := 0
				for _, b := range s {
					total += bits.OnesCount(uint(b))

nit: should this be bits.OnesCount32(uint32(b)), since iterating over a string always returns int32s (which are aliased to runes)?

---

pkg/sql/sem/eval/testdata/eval/builtins line 102 at r1 (raw file):

eval
bit_count('1111111111'::bit(10))

can you also add testing for the bytea overload?

[andrew-delph]

@rafiss Thank you for your comments.

1. I did remove the string overload as it doesn't make sense for the use case hence postgres doesn't include it.
2. You are right. It is now revised to convert it to a []byte and then use bits.OnesCount8 for counting
3. Tests from postgres are included :)

[rafiss]

this lgtm! thank you for your contribution

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)