opt: don't fold sub-operators with null operands during type check

[DrewKimball]

This commit prevents type-checking from replacing expressions like NULL = ANY(...) with NULL. This is necessary because in the case when the right operand is an empty array, the result of the expression is False instead of NULL. It is not always possible to know what the right operand will evaluate to, and constant folding can be handled during normalization anyway.

Fixes #37793

Release note (bug fix): Fixed a long-standing and rare bug in evaluation of ANY, SOME, and ALL sub-operators that would cause expressions like NULL = ANY(ARRAY[]::INT[]) to return NULL instead of False.

[cockroach-teamcity]

This change is 

[msirek]

Very nice!

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

---

pkg/sql/sem/tree/type_check.go line 2106 at r1 (raw file):

		rightReturn := rightTyped.ResolvedType()
		if cmpTypeLeft.Family() == types.UnknownFamily || rightReturn.Family() == types.UnknownFamily {
			return leftTyped, rightTyped, nil, true /* alwaysNull */, nil

Can we still short-circuit return here if one side is UnknownFamily and the other side is anything except ArrayFamily or TupleFamily?

[mgartner]

Reviewed 2 of 3 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @msirek)

---

pkg/sql/sem/tree/type_check.go line 2106 at r1 (raw file):

Previously, msirek (Mark Sirek) wrote…

Can we still short-circuit return here if one side is UnknownFamily and the other side is anything except ArrayFamily or TupleFamily?

We error below if the right side is not of type Tuple or Array.

[DrewKimball]

Can we still short-circuit return here if one side is UnknownFamily and the other side is anything except ArrayFamily or TupleFamily?

We error below if the right side is not of type Tuple or Array.

Turns out we actually have to check if the right side is NULL, since it can be an untyped NULL and we don't want to error for that case. In that case, we can safely return NULL.

[DrewKimball]

TFTRs!

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from 35cab4c to blathers/backport-release-22.1-97948: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.

---

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

[kolloch]

In cockroachdb demo with v22.2.8:

demo@127.0.0.1:26257/movr> select * from public.vehicle_location_histories WHERE (city, ride_id) = ANY(ARRAY(SELECT ROW(city, ride_id) FROM public.vehicle_location_histories LIMIT 1))

;
    city    |               ride_id                |        timestamp        | lat  | long
------------+--------------------------------------+-------------------------+------+-------
  amsterdam | ab020c49-ba5e-4800-8000-00000000014e | 2019-01-02 03:04:05.671 | -172 |  -52
(1 row)

In cockroachdb v23.1.12:

demo@127.0.0.1:26257/movr> select * from public.vehicle_location_histories WHERE (city, ride_id) = ANY(ARRAY(SELECT ROW(city, ride_id) FROM public.vehicle_location_histories LIMIT  
                        -> 1))                                                                                                                                                       
                        -> ;                                                                                                                                                         
ERROR: subquery has too few columns
SQLSTATE: 42601

Since this is a relatively recent change and some of our queries broke between v22.2.8 and v22.3.1, could this be related?

[DrewKimball]

Since this is a relatively recent change and some of our queries broke between v22.2.8 and v22.3.1, could this be related?

Hi @kolloch, thanks for the report. I don't think it was this change that caused the regression, but may have been a related type-checking fix (#101975). For now, I've linked your repro to #102017, since it seems likely to be the same root cause.