-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Coral-Hive] Simplify IN operator #410
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/** | ||
* Copyright 2018-2021 LinkedIn Corporation. All rights reserved. | ||
* Copyright 2018-2023 LinkedIn Corporation. All rights reserved. | ||
* Licensed under the BSD-2 Clause license. | ||
* See LICENSE in the project root for license information. | ||
*/ | ||
|
@@ -34,6 +34,7 @@ | |
import org.apache.calcite.sql.validate.SqlValidatorImpl; | ||
import org.apache.calcite.sql.validate.SqlValidatorScope; | ||
|
||
import static org.apache.calcite.sql.parser.SqlParserPos.ZERO; | ||
import static org.apache.calcite.util.Static.*; | ||
|
||
|
||
|
@@ -47,6 +48,8 @@ | |
* different from set of OR predicates | ||
* 2. In some cases, calcite turns IN clause to INNER JOIN query on a set of values. We | ||
* again want to prevent this conversion. | ||
* Unlike Calcite's IN operator, whose operands are (expr, values), this operator's operands | ||
* are (expr, values[0], values[1], ...), where (exp) is the expression preceding IN operator. | ||
*/ | ||
public class HiveInOperator extends SqlSpecialOperator { | ||
|
||
|
@@ -73,46 +76,42 @@ public void unparse(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) | |
writer.endList(listFrame); | ||
} | ||
|
||
// copied from Calcite' SqlInOperator | ||
// Adapts the parent's behavior to allow IN to have any number of arguments (i.e., list elements). | ||
public RelDataType deriveType(SqlValidator validator, SqlValidatorScope scope, SqlCall call) { | ||
final List<SqlNode> operands = call.getOperandList(); | ||
assert operands.size() == 2; | ||
final SqlNode left = operands.get(0); | ||
final SqlNode right = operands.get(1); | ||
final SqlNodeList right = new SqlNodeList(ZERO); | ||
for (int i = 1; i < operands.size(); i++) { | ||
right.add(operands.get(i)); | ||
} | ||
|
||
final RelDataTypeFactory typeFactory = validator.getTypeFactory(); | ||
RelDataType leftType = validator.deriveType(scope, left); | ||
RelDataType rightType; | ||
|
||
// Derive type for RHS. | ||
if (right instanceof SqlNodeList) { | ||
// Handle the 'IN (expr, ...)' form. | ||
List<RelDataType> rightTypeList = new ArrayList<>(); | ||
SqlNodeList nodeList = (SqlNodeList) right; | ||
for (int i = 0; i < nodeList.size(); i++) { | ||
SqlNode node = nodeList.get(i); | ||
RelDataType nodeType = validator.deriveType(scope, node); | ||
rightTypeList.add(nodeType); | ||
} | ||
rightType = typeFactory.leastRestrictive(rightTypeList); | ||
|
||
// First check that the expressions in the IN list are compatible | ||
// with each other. Same rules as the VALUES operator (per | ||
// SQL:2003 Part 2 Section 8.4, <in predicate>). | ||
if (null == rightType && validator.isTypeCoercionEnabled()) { | ||
// Do implicit type cast if it is allowed to. | ||
rightType = validator.getTypeCoercion().getWiderTypeFor(rightTypeList, true); | ||
} | ||
if (null == rightType) { | ||
throw validator.newValidationError(right, RESOURCE.incompatibleTypesInList()); | ||
} | ||
|
||
// Record the RHS type for use by SqlToRelConverter. | ||
((SqlValidatorImpl) validator).setValidatedNodeType(nodeList, rightType); | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the else part not required anymore because for Could you also add a UT / point me to a UT which validates that type derivation won't fail for those SQLs due to this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// Handle the 'IN (query)' form. | ||
rightType = validator.deriveType(scope, right); | ||
List<RelDataType> rightTypeList = new ArrayList<>(); | ||
for (int i = 0; i < right.size(); i++) { | ||
SqlNode node = right.get(i); | ||
RelDataType nodeType = validator.deriveType(scope, node); | ||
rightTypeList.add(nodeType); | ||
} | ||
rightType = typeFactory.leastRestrictive(rightTypeList); | ||
|
||
// First check that the expressions in the IN list are compatible | ||
// with each other. Same rules as the VALUES operator (per | ||
// SQL:2003 Part 2 Section 8.4, <in predicate>). | ||
if (null == rightType && validator.isTypeCoercionEnabled()) { | ||
// Do implicit type cast if it is allowed to. | ||
rightType = validator.getTypeCoercion().getWiderTypeFor(rightTypeList, true); | ||
} | ||
if (null == rightType) { | ||
throw validator.newValidationError(right, RESOURCE.incompatibleTypesInList()); | ||
} | ||
|
||
// Record the RHS type for use by SqlToRelConverter. | ||
((SqlValidatorImpl) validator).setValidatedNodeType(right, rightType); | ||
|
||
SqlCallBinding callBinding = new SqlCallBinding(validator, scope, call); | ||
// Coerce type first. | ||
if (callBinding.getValidator().isTypeCoercionEnabled()) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of refactoring this operator, can you also rename this operator to
CoralINOperator
?