release-20.2: colfetcher: fix NULL checks during unique index decoding

[mgartner]

Backport 1/1 commits from #68071.

/cc @cockroachdb/release

Release justification: This backport fixes a correctness bug in the
vectorized execution engine. See the release note for details.

---

colfetcher must detect NULL values in unique secondary index keys on
tables with multiple column families in order to determine whether
consecutive KVs belongs to the same row or different rows. Previously,
only the columns in the key that were needed by the query were decoded
and checked for NULL. This caused incorrect query results when NULL
column values were not detected because those columns were not needed.

This commit fixes the issue by checking all columns for NULL when
decoding unique secondary index keys on tables with multiple column
families.

Fixes #66706

Release note (bug fix): A bug has been fix that caused incorrect query
results when querying tables with multiple column families and unique
secondary indexes. The bug only occurred if 1) vectorized execution was
enabled for the query, 2) the query scanned a unique secondary index
that contained columns from more than one column family, and 3) rows
fetched by the query contained NULL values for some of the indexed
columns. This bug was present since version 20.1.

[blathers-crl]

Thanks for opening a backport.

Please check the backport criteria before merging:

• Patches should only be created for serious issues.
• Patches should not break backwards-compatibility.
• Patches should change as little code as possible.
• Patches should not change on-disk formats or node communication protocols.
• Patches should not add new functionality.

If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.

• There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
• The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
• New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
• The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

• What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
• Will this work in a cluster of mixed patch versions? Did we test that?
• If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

[cockroach-teamcity]

This change is 

[mgartner]

I added this because I believe it is necessary in 20.2 to ensure that the queries below use the vectorized engine.

[yuzefovich]

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

---

pkg/sql/logictest/testdata/logic_test/vectorize, line 1298 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

I added this because I believe it is necessary in 20.2 to ensure that the queries below use the vectorized engine.

I think on 20.2 CREATE TABLE and INSERT might not go through vectorized (weren't supported yet), so we want to have this right before the SELECT query, and right after we want RESET vectorize;.

[mgartner]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yuzefovich)

---

pkg/sql/logictest/testdata/logic_test/vectorize, line 1298 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

I think on 20.2 CREATE TABLE and INSERT might not go through vectorized (weren't supported yet), so we want to have this right before the SELECT query, and right after we want RESET vectorize;.

Thanks! Done.