colexec: add support for Concat binary operator

[yongyanglai]

Previously, the vectorized engine lacked support for concat binary
operator.
This commit added this support. I implemented concat for bytes
referrering to same operator for row-oriented egine. Since there was
already a "getBinOpAssignFunc" implemented for "datumCustomizer", I
just registered the output type for Concat operator for datum type, so
that Concat operator for datum type would be generated as expected.

Resolves: #49466
See also: #49463

[cockroach-teamcity]

All committers have signed the CLA.

[cockroach-teamcity]

This change is 

[yuzefovich]

Thanks for working on this! The approach looks good to me, but I have a couple of nits and ask to remove some code duplication.

nit: we usually do not mention the issue numbers in the commit message and add it manually to the PR description. This way the issues don't get a spam of updates every time the commit is force-pushed.

Reviewed 4 of 4 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yongyanglai)

---

pkg/sql/colexec/execgen/cmd/execgen/overloads_base.go, line 817 at r1 (raw file):

		}
	}
	registerTypeCustomizer(typePair{typeconv.DatumVecCanonicalTypeFamily, anyWidth, typeconv.DatumVecCanonicalTypeFamily, anyWidth}, datumCustomizer{})

This is not necessary because we have already registered the type customizer for these typePairs above.

---

pkg/sql/colexec/execgen/cmd/execgen/overloads_bin.go, line 196 at r1 (raw file):

		if op.overloadBase.BinOp == tree.Concat {
			// We expect the target to be of the form "projVec[idx]", however,
			// datumVec doesn't support indexing, so we need to translate that into

nit: it's not datumVec but coldata.Bytes.

Since we're reusing this code, we should extract it into a helper function (maybe named something like parseNonIndexableTargetElem that returns vecVariable and idxVariable).

---

pkg/sql/logictest/testdata/logic_test/vectorize_overloads, line 210 at r1 (raw file):

query T
EXPLAIN (Vec) SELECT _json || _json FROM many_types

nit: s/Vec/VEC/g for consistency (and below).

[yongyanglai]

TFR
PTAL
I have made some changes according to your reviews.

[yuzefovich]

A couple more things, and then I think it is good to go.

nit: in the commit message, you don't need to specify release note category if there is no release note, simply using Release note: None would be sufficient. However, in this case I would actually add a release note since it is a user-visible change, probably something like Release note (sql change): Vectorized execution engine now supports "Concat" ("||") operator.

Reviewed 3 of 3 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yongyanglai)

---

pkg/sql/colexec/execgen/cmd/execgen/overloads_bin.go, line 204 at r2 (raw file):

				r = append(r, %s...)
				r = append(r, %s...)
				%s.Set(%s, r)

You should replace this line with %s and use set() function to generate the necessary Set operation code. Look at executeBinOpOnDatums for an example. This will allow for an easier update in the future if for some reason we decide to change Bytes interface.

---

pkg/sql/colexec/execgen/cmd/execgen/overloads_bin.go, line 687 at r2 (raw file):

}

//Some target element has form "caller[index]", however, types like Bytes and dataumVec

nit: we usually have a space after comment symbol, i.e. // Some target...
nit: s/dataumVec/datumVec/g

---

pkg/sql/colexec/execgen/cmd/execgen/overloads_bin.go, line 688 at r2 (raw file):

//Some target element has form "caller[index]", however, types like Bytes and dataumVec
//doesn't support indexing, we need to translate that into a set operation.This method

nit: s/doesn't/don't/.

[blathers-crl]

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

• Please ensure your pull request description contains a release note - this can be the same as the one in your commit message.
• When CI has completed, please ensure no errors have appeared.

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

[yongyanglai]

Thanks for your reviews!
I have committed some new changes
RFAL

[yuzefovich]

Thanks! I just have one last nit.

Reviewed 1 of 1 files at r3.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @yongyanglai)

---

pkg/sql/colexec/execgen/cmd/execgen/overloads_bin.go, line 687 at r3 (raw file):

}

// Some target element has form "caller[index]", however, types like Bytes and dataumVec

nit: it should be datumVec instead of dataumVec.

[blathers-crl]

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

• Please ensure your pull request description contains a release note - this can be the same as the one in your commit message.
• When CI has completed, please ensure no errors have appeared.

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

[yongyanglai]

Thanks, I have fixed this typo now
RFAL

[yuzefovich]

Thanks for your contribution!

bors r=yuzefovich

Reviewed 1 of 1 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

[craig]

Build succeeded

• GitHub CI (Cockroach)