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


##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -4871,61 +4872,105 @@ 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);
+  }
+
+  /** 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 are not converted over the source alone, so that their
+   * sub-queries are expanded on top of the merge input and their field
+   * references stay valid.
+   *
+   * @return projection whose expressions are the inserted row, in the order of
+   * the target columns; its input is the merge input, possibly extended by
+   * expanded sub-queries */
+  private Project convertMergeInsert(SqlInsert insertCall, RelNode mergeInput) 
{
+    final SqlSelect insertSource = (SqlSelect) insertCall.getSource();
+    final Blackboard bb =
+        createBlackboard(validator().getSelectScope(insertSource), null, 
false);
+    bb.setRoot(mergeInput, true);
+    callStack.push(insertCall);
+    convertSelectList(bb, bb, insertSource, ImmutableList.of(),
+        ImmutableList.of(), null);
+    callStack.pop();
+    final Project valuesProject = (Project) bb.root();
+
+    // Order the values by the target columns and add the defaults. The
+    // projection is built by RelBuilder on top of the values projection, so
+    // it may be the values projection itself (identity), a projection over
+    // it, or a projection merged with it (over its input).
+    final RelNode columns = convertColumnList(insertCall, valuesProject);
+    if (columns == valuesProject) {
+      return valuesProject;
+    }
+    final Project columnsProject = (Project) columns;
+    final List<RexNode> exprs;
+    if (columnsProject.getInput() == valuesProject) {
+      exprs =
+          RelOptUtil.pushPastProject(columnsProject.getProjects(),
+              valuesProject);
+    } else if (columnsProject.getInput() == valuesProject.getInput()) {
+      exprs = columnsProject.getProjects();
+    } else {
+      throw new AssertionError("unexpected input of insert columns: "
+          + columnsProject.getInput());
+    }
+    return LogicalProject.create(valuesProject.getInput(), ImmutableList.of(),
+        exprs, columnsProject.getRowType().getFieldNames(),
+        valuesProject.getVariablesSet());
+  }
 
-    return LogicalTableModify.create(targetTable, catalogReader,
-        relBuilder.build(), LogicalTableModify.Operation.MERGE,
-        targetColumnNameList, null, false);
+  /** Creates the modification of a MERGE. As for INSERT, a modifiable table

Review Comment:
   Agreed, the javadoc was misleading. The converter itself does not depend on 
JDBC. Like `createModify`, which handles INSERT, UPDATE and DELETE, 
`createMergeModify` only calls the generic `ModifiableTable.toModificationRel`, 
so a table can create its own `TableModify`. What a particular table does 
there, `JdbcTable` included, is up to its adapter. Before this PR, MERGE 
already went through the same call via `convertInsert` → `createModify`, so 
this keeps the existing behavior. I removed the JDBC-specific text from the 
javadoc.



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