[Coral-Hive] Simplify IN operator #410
Conversation
wmoustafa
changed the title
There was a problem hiding this comment.
Thanks for the PR @wmoustafa !
Please also update the PR description with the results from i-tests when the results are available.
|
|
||
| // Record the RHS type for use by SqlToRelConverter. | ||
| ((SqlValidatorImpl) validator).setValidatedNodeType(nodeList, rightType); | ||
| } else { |
There was a problem hiding this comment.
is the else part not required anymore because for IN (query) type SQLs, we use Calcite's IN operator here?
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.
right is now explicitly built as a SqlNodeList using the list of operands, so there is no case where right isn't an instance of SqlNodeList.
| @@ -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 { | |||
There was a problem hiding this comment.
As part of refactoring this operator, can you also rename this operator to CoralINOperator?
What changes are proposed in this pull request, and why are they necessary?
This PR simplifies how Coral handles the
expr IN (values)operator.Previously, the following set of transformations took place:
1- Hive's
ParseTreeBuilderprocesses theINoperator as part ofvisitFunctionInternal, specifically, passing the list of(expr, value_0, value_1, ...)as operands.2- This maps to
HiveInOperatorwhen looked up from the function registry. This operator overridescreateCallin such a way that makes the operands be on the format(expr, values)(i.e., instead of N operands, two operands are used with the second being the list of values).3-
HiveConvertletTableforINoperator convertsIN SqlNodeto aRexNodewith operands on the format(expr, value_0, value_1, ...)again.This PR sticks to the operand format
(expr, value_0, value_1, ...)across the board, simplifying the back and forth transformations, and eliminating the need for theHiveConvertletTablein theINoperator case.How was this patch tested?
./gradlew build