[GitHub] [calcite] asolimando commented on a change in pull request #2613: [CALCITE-4894] MV rewriting fails for expressions with more than a field reference in the view/query projection

[GitBox <gi...@apache.org>]

asolimando commented on a change in pull request #2613:
URL: https://github.com/apache/calcite/pull/2613#discussion_r762077681



##########
File path: core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java
##########
@@ -1036,21 +1037,36 @@ protected NodeLineage generateSwapTableColumnReferencesLineage(
     final Map<RexNode, Integer> exprsLineage = new HashMap<>();
     final Map<RexNode, Integer> exprsLineageLosslessCasts = new HashMap<>();
     for (int i = 0; i < nodeExprs.size(); i++) {
-      final Set<RexNode> s = mq.getExpressionLineage(node, nodeExprs.get(i));
-      if (s == null) {
+      final Set<RexNode> lineages = mq.getExpressionLineage(node, nodeExprs.get(i));
+      if (lineages == null) {
         // Next expression
         continue;
       }
-      // We only support project - filter - join, thus it should map to
-      // a single expression
-      assert s.size() == 1;
-      // Rewrite expr. First we swap the table references following the table
-      // mapping, then we take first element from the corresponding equivalence class
-      final RexNode e = RexUtil.swapTableColumnReferences(rexBuilder,
-          s.iterator().next(), tableMapping, ec.getEquivalenceClassesMap());
-      exprsLineage.put(e, i);
-      if (RexUtil.isLosslessCast(e)) {
-        exprsLineageLosslessCasts.put(((RexCall) e).getOperands().get(0), i);
+      final RexNode expr = nodeExprs.get(i);
+      if (SqlKind.AND == expr.getKind() || SqlKind.OR == expr.getKind()) {

Review comment:
       You are right, we can go for a more general solution and support all complex expressions like the one you mentioned. I will also add one or more tests for that.
   
   Regarding `mq.expressionsLineage` I tend to agree with you, it speaks of returning the `set of resulting expressions equivalent to the input expression`, but then it splits it into multiple expressions.
   
   If `mq.expressionsLineage` is broken then maybe we don't have an issue here at all in the first place. I can't see any unit tests for this part of the code, it's hard to tell the original intention behind the method.




-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org