Validate literals in expression analyzer #10720
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kasiafi
reviewed
Jan 21, 2022
I thought about that, i chose not to, because i already have resolved coercion at hand, so |
findepi
force-pushed
the
findepi/validate-literals-in-expression-analyzer-d1fe12
branch
from
4ee442d to
d3670d8
Compare
switched over to LiteralInterpreter, as otherwise JSON verification would be cumbersome
added |
findepi
commented
Jan 21, 2022
| // json | ||
| assertFails("SELECT JSON ''") | ||
| .hasErrorCode(INVALID_LITERAL); | ||
| // TODO https://github.com/trinodb/trino/issues/10724 these should fail |
findepi
force-pushed
the
findepi/validate-literals-in-expression-analyzer-d1fe12
branch
from
d3670d8 to
bc3af3f
Compare
findepi
commented
Jan 21, 2022
Comment on lines
195
to
196
| // TODO (https://github.com/trinodb/trino/issues/10732) the expressions here may fail or not, depending whether they are wrapped | ||
| // should deterministically fail with NUMERIC_VALUE_OUT_OF_RANGE |
findepi
force-pushed
the
findepi/validate-literals-in-expression-analyzer-d1fe12
branch
from
bc3af3f to
bc0899b
Compare
findepi
commented
Jan 21, 2022
| @@ -5440,7 +5440,7 @@ public void testTry() | |||
| assertQuery("SELECT apply(5 + RANDOM(1), x -> x + TRY(1 / 0))", "SELECT NULL"); | |||
|
|
|||
| // test try with invalid JSON | |||
| assertQuery("SELECT JSON_FORMAT(TRY(JSON 'INVALID'))", "SELECT NULL"); | |||
| assertQueryFails("SELECT JSON_FORMAT(TRY(JSON 'INVALID'))", "line 1:24: 'INVALID' is not a valid json literal"); | |||
There was a problem hiding this comment.
This is a behavioral change, but IMO justified.
| @@ -5455,7 +5455,7 @@ public void testTry() | |||
| assertQuery("SELECT COALESCE(TRY(CAST(CONCAT('a', CAST(123 AS VARCHAR)) AS BIGINT)), 0)", "SELECT 0L"); | |||
| assertQuery("SELECT 123 + TRY(ABS(-9223372036854775807 - 1))", "SELECT NULL"); | |||
| assertQuery("SELECT JSON_FORMAT(TRY(JSON '[]')) || '123'", "SELECT '[]123'"); | |||
| assertQuery("SELECT JSON_FORMAT(TRY(JSON 'INVALID')) || '123'", "SELECT NULL"); | |||
| assertQueryFails("SELECT JSON_FORMAT(TRY(JSON 'INVALID')) || '123'", "line 1:24: 'INVALID' is not a valid json literal"); | |||
There was a problem hiding this comment.
This is a behavioral change -- see above.
kasiafi
approved these changes
Jan 24, 2022
|
rebased to resolve conflict with #10737 |
findepi
force-pushed
the
findepi/validate-literals-in-expression-analyzer-d1fe12
branch
2 times, most recently
from
0a7d720 to
26dbcac
Compare
|
To be rebased after #10783 is merged. |
findepi
force-pushed
the
findepi/validate-literals-in-expression-analyzer-d1fe12
branch
from
26dbcac to
4d65d6c
Compare
|
rebased after #10783 merged (no changes yet) |
As a result, optimizer can safely assume a `Literal` to represent a value of given type. Also, queries with invalid literal values should be guaranteed to fail, regardless of expression pruning.
findepi
force-pushed
the
findepi/validate-literals-in-expression-analyzer-d1fe12
branch
from
4d65d6c to
d4dc855
Compare
findepi
deleted the
findepi/validate-literals-in-expression-analyzer-d1fe12
branch
This was referenced Jan 27, 2022
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a result, optimizer can safely assume a
Literalto represent avalue of given type. Also, queries with invalid literal values should be
guaranteed to fail, regardless of expression pruning.
Fixes #10719
Fixes #10755