zabetak commented on PR #4253: URL: https://github.com/apache/hive/pull/4253#issuecomment-1521522423
> This is a good contribution to Calcite and the patch looks good to me. > But normally we should not have LogicalCorrelate after the decorrelation step in Hive. Did you check why it is failed in case of the query mentioned in the jira? The decorrelator does not cope well with values. Initially, it will return null when it encounters the `LogicalValues` (https://github.com/apache/hive/blob/59058c65457fb7ab9d8575a555034e6633962661/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java#L471) and then it will bail out when treating the `LogicalCorrelate` (https://github.com/apache/hive/blob/59058c65457fb7ab9d8575a555034e6633962661/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java#L1247) since one of the inputs is not rewritten. The problem is still there in recent Calcite (CALCITE-5568). We can/could treat the problem in the decorrelator in two ways: a) By changing the `decorrelateRel` method(s); for sure the one for `decorrelateRel(LogicalCorrelate)` b) Putting the new rules and some more (reductions) inside `HiveRelDecorrelator#decorrelate(root)` (https://github.com/apache/hive/blob/59058c65457fb7ab9d8575a555034e6633962661/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java#L238). @kasakrisz Let me know if you prefer that we explore one of the alternative paths mentioned above now or if we could postpone for a follow-up. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
