Skip to content

Commit dc68abf

Browse files
authored
fix: Add type check for ReplaceConstantVariableReferencesWithConstants (#26458)
## Description As title. So this optimizer is trying to connect variables and its value if the variable will be a constant due to filter/project assignment etc. In production, we see query which have this optimizer failing because the variable is of type varchar but the constant is of type varchar(4), and it fails when we do the assert that the constant and variable are of the same type in the end of the optimization. To avoid query failure, this PR adds a check before adding the varchar and constant mapping. ## Motivation and Context ## Impact ## Test Plan I tried to reproduce this with open source dataset, but not easy as it is due to some internal function related stuff. Anyway this change is a safe change without any adverse impact. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes ``` == NO RELEASE NOTE == ```
1 parent 4ef4152 commit dc68abf

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/ReplaceConstantVariableReferencesWithConstants.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public PlanNodeWithConstant visitFilter(FilterNode node, Void context)
202202
if (newConstantMap.containsKey(variable) && !newConstantMap.get(variable).equals(constant)) {
203203
return new PlanNodeWithConstant(replaceChildren(node, ImmutableList.of(rewrittenChild.getPlanNode())), ImmutableMap.of());
204204
}
205-
if (!constant.isNull()) {
205+
if (!constant.isNull() && variable.getType().equals(constant.getType())) {
206206
planChanged = true;
207207
newConstantMap.put(variable, constant);
208208
}
@@ -235,7 +235,7 @@ public PlanNodeWithConstant visitProject(ProjectNode node, Void context)
235235
for (Map.Entry<VariableReferenceExpression, RowExpression> entry : newProjectNode.getAssignments().getMap().entrySet()) {
236236
if (entry.getValue() instanceof ConstantExpression && isSupportedType(entry.getKey()) && isSupportedType(entry.getValue())) {
237237
ConstantExpression constantExpression = (ConstantExpression) entry.getValue();
238-
if (!constantExpression.isNull()) {
238+
if (!constantExpression.isNull() && entry.getKey().getType().equals(constantExpression.getType())) {
239239
planChanged = true;
240240
newConstantMap.put(entry.getKey(), constantExpression);
241241
}

0 commit comments

Comments
 (0)