joooger commented on code in PR #5332:
URL: https://github.com/apache/calcite/pull/5332#discussion_r4344627453


##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -4884,61 +4885,111 @@ private RelNode convertMerge(SqlMerge call) {
             () -> "sourceSelect for " + call);
     RelNode mergeSourceRel = convertSelect(sourceSelect, false);
 
-    // then, convert the insert statement so we can get the insert
-    // values expressions
-    SqlInsert insertCall = call.getInsertCall();
-    int nLevel1Exprs = 0;
-    List<RexNode> level1InsertExprs = null;
-    List<RexNode> level2InsertExprs = null;
-    if (insertCall != null) {
-      RelNode insertRel = convertInsert(insertCall);
-
-      // if there are 2 level of projections in the insert source, combine
-      // them into a single project; level1 refers to the topmost project;
-      // the level1 projection contains references to the level2
-      // expressions, except in the case where no target expression was
-      // provided, in which case, the expression is the default value for
-      // the column; or if the expressions directly map to the source
-      // table
-      level1InsertExprs =
-          ((LogicalProject) insertRel.getInput(0)).getProjects();
-      if (insertRel.getInput(0).getInput(0) instanceof LogicalProject) {
-        level2InsertExprs =
-            ((LogicalProject) insertRel.getInput(0).getInput(0))
-                .getProjects();
-      }
-      nLevel1Exprs = level1InsertExprs.size();
-    }
-
-    LogicalJoin join = (LogicalJoin) mergeSourceRel.getInput(0);
-    int nSourceFields = join.getLeft().getRowType().getFieldCount();
+    // The input of the source projection is the join of the source and the
+    // target, possibly wrapped by the joins or correlates that expand the
+    // sub-queries of the SET expressions. Source fields come first in all of
+    // them.
+    RelNode mergeInput = mergeSourceRel.getInput(0);
+    final SqlNode sourceFrom =
+        requireNonNull(sourceSelect.getFrom(), () -> "from for " + 
sourceSelect);
+    final SqlNode sourceRef = ((SqlJoin) sourceFrom).getLeft();
+    final SqlValidatorNamespace sourceNamespace =
+        requireNonNull(validator().getNamespace(sourceRef),
+            () -> "namespace for " + sourceRef);
+    final int nSourceFields = sourceNamespace.getRowType().getFieldCount();
+    final Set<CorrelationId> variablesSet =
+        new HashSet<>(((LogicalProject) mergeSourceRel).getVariablesSet());
+
     final List<RexNode> projects = new ArrayList<>();
-    for (int level1Idx = 0; level1Idx < nLevel1Exprs; level1Idx++) {
-      requireNonNull(level1InsertExprs, "level1InsertExprs");
-      if ((level2InsertExprs != null)
-          && (level1InsertExprs.get(level1Idx) instanceof RexInputRef)) {
-        int level2Idx =
-            ((RexInputRef) level1InsertExprs.get(level1Idx)).getIndex();
-        projects.add(level2InsertExprs.get(level2Idx));
-      } else {
-        projects.add(level1InsertExprs.get(level1Idx));
-      }
+    final SqlInsert insertCall = call.getInsertCall();
+    if (insertCall != null) {
+      final Project insertProject = convertMergeInsert(insertCall, mergeInput);
+      projects.addAll(insertProject.getProjects());
+      mergeInput = insertProject.getInput();
+      variablesSet.addAll(insertProject.getVariablesSet());
     }
+
     if (updateCall != null) {
       final LogicalProject project = (LogicalProject) mergeSourceRel;
       projects.addAll(
           Util.skip(project.getProjects(), nSourceFields));
     }
 
-    // Keep the correlation variables set by the source projection: the SET
-    // expressions may contain correlated sub-queries.
-    relBuilder.push(join)
-        .project(projects, ImmutableList.of(), false,
-            ((LogicalProject) mergeSourceRel).getVariablesSet());
+    // Keep the correlation variables: the SET expressions and the insert
+    // values may contain correlated sub-queries.
+    relBuilder.push(mergeInput)
+        .project(projects, ImmutableList.of(), false, variablesSet);
+    return createMergeModify(targetTable, relBuilder.build(),
+        targetColumnNameList);
+  }
 
-    return LogicalTableModify.create(targetTable, catalogReader,
-        relBuilder.build(), LogicalTableModify.Operation.MERGE,
-        targetColumnNameList, null, false);
+  /** Converts the values of the INSERT of a MERGE over the input of the MERGE
+   * source projection, whose first fields are the source fields.
+   *
+   * <p>The values reference the source fields, so they are converted over the

Review Comment:
   Done, I removed the description and kept only the @return.



##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -4884,61 +4885,111 @@ private RelNode convertMerge(SqlMerge call) {
             () -> "sourceSelect for " + call);
     RelNode mergeSourceRel = convertSelect(sourceSelect, false);
 
-    // then, convert the insert statement so we can get the insert
-    // values expressions
-    SqlInsert insertCall = call.getInsertCall();
-    int nLevel1Exprs = 0;
-    List<RexNode> level1InsertExprs = null;
-    List<RexNode> level2InsertExprs = null;
-    if (insertCall != null) {
-      RelNode insertRel = convertInsert(insertCall);
-
-      // if there are 2 level of projections in the insert source, combine
-      // them into a single project; level1 refers to the topmost project;
-      // the level1 projection contains references to the level2
-      // expressions, except in the case where no target expression was
-      // provided, in which case, the expression is the default value for
-      // the column; or if the expressions directly map to the source
-      // table
-      level1InsertExprs =
-          ((LogicalProject) insertRel.getInput(0)).getProjects();
-      if (insertRel.getInput(0).getInput(0) instanceof LogicalProject) {
-        level2InsertExprs =
-            ((LogicalProject) insertRel.getInput(0).getInput(0))
-                .getProjects();
-      }
-      nLevel1Exprs = level1InsertExprs.size();
-    }
-
-    LogicalJoin join = (LogicalJoin) mergeSourceRel.getInput(0);
-    int nSourceFields = join.getLeft().getRowType().getFieldCount();
+    // The input of the source projection is the join of the source and the
+    // target, possibly wrapped by the joins or correlates that expand the
+    // sub-queries of the SET expressions. Source fields come first in all of
+    // them.
+    RelNode mergeInput = mergeSourceRel.getInput(0);
+    final SqlNode sourceFrom =
+        requireNonNull(sourceSelect.getFrom(), () -> "from for " + 
sourceSelect);
+    final SqlNode sourceRef = ((SqlJoin) sourceFrom).getLeft();
+    final SqlValidatorNamespace sourceNamespace =
+        requireNonNull(validator().getNamespace(sourceRef),
+            () -> "namespace for " + sourceRef);
+    final int nSourceFields = sourceNamespace.getRowType().getFieldCount();
+    final Set<CorrelationId> variablesSet =
+        new HashSet<>(((LogicalProject) mergeSourceRel).getVariablesSet());
+
     final List<RexNode> projects = new ArrayList<>();
-    for (int level1Idx = 0; level1Idx < nLevel1Exprs; level1Idx++) {
-      requireNonNull(level1InsertExprs, "level1InsertExprs");
-      if ((level2InsertExprs != null)
-          && (level1InsertExprs.get(level1Idx) instanceof RexInputRef)) {
-        int level2Idx =
-            ((RexInputRef) level1InsertExprs.get(level1Idx)).getIndex();
-        projects.add(level2InsertExprs.get(level2Idx));
-      } else {
-        projects.add(level1InsertExprs.get(level1Idx));
-      }
+    final SqlInsert insertCall = call.getInsertCall();
+    if (insertCall != null) {
+      final Project insertProject = convertMergeInsert(insertCall, mergeInput);
+      projects.addAll(insertProject.getProjects());
+      mergeInput = insertProject.getInput();
+      variablesSet.addAll(insertProject.getVariablesSet());
     }
+
     if (updateCall != null) {
       final LogicalProject project = (LogicalProject) mergeSourceRel;
       projects.addAll(
           Util.skip(project.getProjects(), nSourceFields));
     }
 
-    // Keep the correlation variables set by the source projection: the SET
-    // expressions may contain correlated sub-queries.
-    relBuilder.push(join)
-        .project(projects, ImmutableList.of(), false,
-            ((LogicalProject) mergeSourceRel).getVariablesSet());
+    // Keep the correlation variables: the SET expressions and the insert
+    // values may contain correlated sub-queries.
+    relBuilder.push(mergeInput)
+        .project(projects, ImmutableList.of(), false, variablesSet);
+    return createMergeModify(targetTable, relBuilder.build(),
+        targetColumnNameList);
+  }
 
-    return LogicalTableModify.create(targetTable, catalogReader,
-        relBuilder.build(), LogicalTableModify.Operation.MERGE,
-        targetColumnNameList, null, false);
+  /** Converts the values of the INSERT of a MERGE over the input of the MERGE
+   * source projection, whose first fields are the source fields.
+   *
+   * <p>The values reference the source fields, so they are converted over the
+   * merge input: the join of the source and the target, plus the joins or
+   * correlates that expand the sub-queries of the SET expressions. If the
+   * sub-queries of the values are expanded, their joins or correlates are
+   * added on top of the merge input; the source fields stay first, so the
+   * field references of the values stay valid.
+   *
+   * @return A projection whose expressions are the inserted row, in the order

Review Comment:
   Agreed, these are details of convertSelectList. The javadoc now only says 
what the caller relies on: the input of the returned projection starts with the 
fields of mergeInput.



-- 
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]

Reply via email to