[CALCITE-6877] Generate LogicalProject in RelRoot.project() when mapping is not name trivial

Signed-off-by: Niels Pardon <par@zurich.ibm.com>

Close apache#4230

Author: nielspardon

core/src/main/java/org/apache/calcite/rel/RelRoot.java

@@ -164,7 +164,7 @@ public RelNode project(boolean force) {
     if (isRefTrivial()
         && (SqlKind.DML.contains(kind)
             || !force
-            || rel instanceof LogicalProject)) {
+            || (rel instanceof LogicalProject && isNameTrivial()))) {
       return rel;
     }
     final List<RexNode> projects = new ArrayList<>(fields.size());

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

@@ -27,12 +27,14 @@
 import org.apache.calcite.rel.RelFieldCollation.Direction;
 import org.apache.calcite.rel.RelFieldCollation.NullDirection;
 import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelRoot;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.hint.HintPredicates;
 import org.apache.calcite.rel.hint.HintStrategyTable;
 import org.apache.calcite.rel.hint.RelHint;
 import org.apache.calcite.rel.logical.LogicalAggregate;
 import org.apache.calcite.rel.logical.LogicalFilter;
+import org.apache.calcite.rel.logical.LogicalProject;
 import org.apache.calcite.rel.rules.AggregateJoinTransposeRule;
 import org.apache.calcite.rel.rules.AggregateProjectMergeRule;
 import org.apache.calcite.rel.rules.CoreRules;
@@ -42,14 +44,18 @@
 import org.apache.calcite.rel.rules.PruneEmptyRules;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
 import org.apache.calcite.rel.type.RelDataTypeSystemImpl;
+import org.apache.calcite.rel.type.RelRecordType;
 import org.apache.calcite.runtime.FlatLists;
 import org.apache.calcite.runtime.Hook;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlDialect;
 import org.apache.calcite.sql.SqlDialect.DatabaseProduct;
+import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlSelect;
 import org.apache.calcite.sql.SqlWriter;
@@ -99,6 +105,7 @@
 
 import java.math.BigDecimal;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -114,7 +121,10 @@
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasToString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
@@ -9315,6 +9325,83 @@ private void checkLiteral2(String expression, String expected) {
   }
 
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6877">[CALCITE-6877]
+   * Generate LogicalProject in RelRoot.project() when mapping is not name trivial</a>. */
+  @Test void testRelRootProjectForceNonNameTrivial() {
+    final SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    final SchemaPlus defaultSchema =
+        CalciteAssert.addSchema(rootSchema, CalciteAssert.SchemaSpec.HR);
+    final FrameworkConfig frameworkConfig = RelBuilderTest.config()
+        .defaultSchema(defaultSchema)
+        .build();
+    final RelBuilder relBuilder = RelBuilder.create(frameworkConfig);
+    final RelNode inputRel = relBuilder.scan("emps")
+        .project(relBuilder.fields(Collections.singletonList("empid"))).build();
+
+    final List<RelDataTypeField> fields =
+        Collections.singletonList(
+            // rename empid to empno via RelRoot
+            new RelDataTypeFieldImpl("empno",
+                inputRel.getRowType().getFieldList().get(0).getIndex(),
+                inputRel.getRowType().getFieldList().get(0).getType()));
+
+    final RelRoot root = RelRoot.of(inputRel, new RelRecordType(fields), SqlKind.SELECT);
+
+    // inner LogicalProject selects one field and RelRoot only has one field
+    assertTrue(root.isRefTrivial());
+
+    // inner LogicalProject has different field name than RelRoot
+    assertFalse(root.isNameTrivial());
+
+    final RelNode project = root.project();
+    assertEquals(inputRel, project);
+
+    // regular project() and force project() are different
+    final RelNode forceProject = root.project(true);
+    assertNotEquals(project, forceProject);
+
+    // new LogicalProject on top of inputRel
+    assertInstanceOf(LogicalProject.class, forceProject);
+    assertEquals(inputRel, forceProject.getInput(0));
+
+    // new LogicalProject renames field
+    if (forceProject instanceof LogicalProject) {
+      assertEquals("empno",
+          ((LogicalProject) forceProject).getNamedProjects().get(0).getValue());
+    }
+  }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6877">[CALCITE-6877]
+   * Generate LogicalProject in RelRoot.project() when mapping is not name trivial</a>. */
+  @Test void testRelRootProjectForceNameTrivial() {
+    final SchemaPlus rootSchema = Frameworks.createRootSchema(true);
+    final SchemaPlus defaultSchema =
+        CalciteAssert.addSchema(rootSchema, CalciteAssert.SchemaSpec.HR);
+    final FrameworkConfig frameworkConfig = RelBuilderTest.config()
+        .defaultSchema(defaultSchema)
+        .build();
+    final RelBuilder relBuilder = RelBuilder.create(frameworkConfig);
+    final RelNode inputRel = relBuilder.scan("emps")
+        .project(relBuilder.fields(Collections.singletonList("empid"))).build();
+
+    final RelRoot root = RelRoot.of(inputRel, SqlKind.SELECT);
+
+    // inner LogicalProject selects one field and RelRoot only has one field
+    assertTrue(root.isRefTrivial());
+
+    // inner LogicalProject has same field name as RelRoot
+    assertTrue(root.isNameTrivial());
+
+    final RelNode project = root.project();
+    assertEquals(inputRel, project);
+
+    // regular project() and force project() are the same
+    final RelNode forceProject = root.project(true);
+    assertEquals(project, forceProject);
+  }
+
   /** Fluid interface to run tests. */
   static class Sql {
     private final CalciteAssert.SchemaSpec schemaSpec;