tree: don't resolve expressions with wildcard types #108892
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained
yuzefovich
added
backport-22.2.x
yuzefovich
force-pushed
the
wildcard-type
branch
from
9f7a25a
to
8034379
Compare
There was a problem hiding this comment.
I agree that it seems worth it to backport, so I added the corresponding labels, but it might be good to give it some baking time on master (the automatic backports should fail anyway).
bors r=rharding6373,yuzefovich
Reviewed 3 of 3 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status:
|
Build failed: |
|
Looks like CI is red, perhaps #108387 changed something related. |
mgartner
force-pushed
the
wildcard-type
branch
from
8034379
to
42ffe0b
Compare
|
Looks like this fixes some tests which didn't match Postgres behavior (the tests I changed in the added commit), while it broke others (the tests I commented-out in the added commit). |
There are various wildcard types that are used during type-checking, but which are not valid during execution. Previously, we only checked for `types.Any` in several places, but there are other wildcard types like `types.AnyEnum` with similar properties. This could lead to an internal panic during execution when this invalid type was resolved. This patch adds a new method `types.T.IsWildcardType()` that checks for all wildcard types, to be used during type-checking. Fixes cockroachdb#83496 Release note (bug fix): Fixed a bug that has existed since before v22.2 which could cause an internal error during distributed execution for an expression like `CASE` that requires its inputs to be the same type with all `NULL` inputs.
DrewKimball
force-pushed
the
wildcard-type
branch
from
42ffe0b
to
536ff94
Compare
There was a problem hiding this comment.
Sorry for the delay, PTAL.
Reviewable status:
pkg/sql/sem/tree/type_check.go line 1712 at r3 (raw file):
if len(expr.Exprs) == 0 { if desiredParam.IsWildcardType() {
The change I made to ArrayExpr type checking was an oversight - only types.Any is used as a wildcard there.
There was a problem hiding this comment.
nit: I'd remove backport 22.2 and 23.1 labels and maybe would add 23.2.
Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status:
DrewKimball
removed
backport-22.2.x
Sure. I'll give it some time to bake before thinking about backporting, since it seems low-priority. |
|
TRTR! bors r+ |
|
Build succeeded: |
There are various wildcard types that are used during type-checking, but which are not valid during execution. Previously, we only checked for
types.Anyin several places, but there are other wildcard types liketypes.AnyEnumwith similar properties. This could lead to an internal panic during execution when this invalid type was resolved. This patch adds a new methodtypes.T.IsWildcardType()that checks for all wildcard types, to be used during type-checking.Fixes #83496
Release note (bug fix): Fixed a bug that has existed since before v22.2 which could cause an internal error during distributed execution for an expression like
CASEthat requires its inputs to be the same type with allNULLinputs.