sql: add the 'every' aggregate function

[rohany]

Fixes #45842.

Release justification: low risk update to functionality
Release note (sql change): This PR adds the every aggregate function.

[cockroach-teamcity]

This change is 

[rohany]

Not sure if there is an easier way of just translating the every function into bool_and -- maybe something in the optimizer can make this easier?

[andy-kimball]

As far as I understand, this function is exactly equivalent to bool_and, so it should definitely be an optimizer thing. I hadn't realized that before. If so, then I'd expect change to be something like:

	case "bit_and", "every":
		return b.factory.ConstructBitAndAgg(args[0])

[rohany]

Sounds good -- editing the distsql infra seemed wrong. I'll still have to make a scalar for it though, right?

[andy-kimball]

No need for a scalar. We'll just create the BitAnd scalar operator directly.

[rohany]

Alright done. Two things though:

• I want to include the distsqlversion bump because of some other aggregates that have been added that need a bump.
• I'm leaning on keeping the every entry in the aggregate funcs map so that a docs entry gets generated for it -- not sure if there is an easy way around that

[andy-kimball]

---

pkg/sql/rowexec/version_history.txt, line 109 at r1 (raw file):

    - Change how DArray datums are hashed for distinct and aggregation operations.
- Version: 28 (MinAcceptedVersion: 28)
    - Some new aggregate functions have been added (BIT_AND, BIT_OR, CORR)

Haven't BIT_AND and BIT_OR been around for a while?

[andy-kimball]

That's fine to keep it in the map.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball, @rohany, and @yuzefovich)

[yuzefovich]

Reviewed 6 of 6 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball and @rohany)

---

pkg/sql/rowexec/version_history.txt, line 109 at r1 (raw file):

Previously, andy-kimball (Andy Kimball) wrote…

Haven't BIT_AND and BIT_OR been around for a while?

For a few months, yes.

I don't think bumping up the version here is strictly required, yet it is beneficial. I believe without the bump the following can occur in a mixed version cluster:

• the gateway node has the newest version that supports BIT_AND, so when the query with this function is issued against the gateway, it thinks everything is great, so it sends out the flow specifications containing this function to remote nodes
• the remote nodes, however, are running an older version which doesn't have the support for BIT_AND, so they will return an error to the gateway because they don't recognize the constant that corresponds to BIT_AND, and the whole query will fail.

The important point is that we do not change the order of aggregate functions (i.e. the respective constants remain unchanged), so there is no change of some "internal" error or a crash due to aggregates mismatch. (If my understanding is incorrect, please let me know :) )

---

pkg/sql/sem/builtins/aggregate_builtins.go, line 148 at r1 (raw file):

	),

	"every": makeBuiltin(aggProps(),

nit: these functions are sorted lexicographically.

[rohany]

Sorry, I misread the dates on the commit that added bit_and and bit_or.

I don't think bumping up the version here is strictly required, yet it is beneficial. I believe without the bump the following can occur in a mixed version cluster:

Yeah, this is what I'm thinking as well.

[yuzefovich]

but I believe we need an approval from a tech lead or from Andy K since we're in stability period.

Reviewed 2 of 2 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andy-kimball and @rohany)

[rohany]

cc @andy-kimball for approval

[andy-kimball]

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @andy-kimball and @rohany)

[rohany]

bors r+

[craig]

Build failed (retrying...)

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)