Upgrade linkedin-calcite-core to 1.21.0.153 and add explicit table alias to solve wrong alias issue

coral-hive/src/main/java/com/linkedin/coral/transformers/CoralRelToSqlNodeConverter.java

@@ -19,6 +19,7 @@
 import org.apache.calcite.rel.core.Project;
 import org.apache.calcite.rel.core.TableScan;
 import org.apache.calcite.rel.core.Uncollect;
+import org.apache.calcite.rel.core.Values;
 import org.apache.calcite.rel.logical.LogicalTableFunctionScan;
 import org.apache.calcite.rel.rel2sql.RelToSqlConverter;
 import org.apache.calcite.rel.type.RelDataType;
@@ -89,6 +90,15 @@ public boolean requireCastOnString() {
          */
         return true;
       }
+
+      /**
+       * Override this method so that there will be explicit and correct table alias for selected fields, which is necessary for
+       * data type derivation on SqlNodes
+       */
+      @Override
+      public boolean hasImplicitTableAlias() {
+        return false;
+      }
     };
   }
 
@@ -150,12 +160,12 @@ public Result visit(TableScan e) {
    */
   @Override
   public Result visit(Correlate e) {
-    final Result leftResult = visitChild(0, e.getLeft());
+    final Result leftResult = visitChild(0, e.getLeft()).resetAlias();
 
     // Add context specifying correlationId has same context as its left child
     correlTableMap.put(e.getCorrelationId(), leftResult.qualifiedContext());
 
-    final Result rightResult = visitChild(1, e.getRight());
+    final Result rightResult = visitChild(1, e.getRight()).resetAlias();
 
     SqlNode rightSqlNode = rightResult.asFrom();
 
@@ -348,6 +358,31 @@ public Result visit(Uncollect e) {
         ImmutableMap.of(projectResult.neededAlias, e.getRowType()));
   }
 
+  /**
+   * Override this method to avoid the duplicated alias for {@link org.apache.calcite.rel.logical.LogicalValues}.
+   * The original {@link org.apache.calcite.rel.rel2sql.SqlImplementor.Result#neededAlias} is `t`, we override
+   * it to be `null`.
+   * So that for the input SQL like `SELECT 1`, the translated SQL will be like:
+   *
+   * SELECT 1
+   * FROM (VALUES  (0)) t (ZERO)
+   *
+   * Without this override, the translated SQL contains duplicated alias `t`:
+   *
+   * SELECT 1
+   * FROM (VALUES  (0)) t (ZERO) t
+   *
+   * which is wrong.
+   *
+   * TODO: Identify and backport the fix from apache-calcite to linkedin-calcite since the duplicated alias issue only happens in linkedin-calcite
+   */
+  @Override
+  public Result visit(Values e) {
+    final Result originalResult = super.visit(e);
+    return new Result(originalResult.node, originalResult.clauses, null, originalResult.neededType,
+        originalResult.aliases);
+  }
+
   private SqlNode generateRightChildForSqlJoinWithLateralViews(BiRel e, Result rightResult) {
     SqlNode rightSqlNode = rightResult.asFrom();
 

coral-spark/src/test/java/com/linkedin/coral/spark/FuzzyUnionViewTest.java

@@ -43,8 +43,8 @@ public void testNoSchemaEvolution() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql =
-        "" + "SELECT *\n" + "FROM fuzzy_union.tablea\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablea";
+    String expectedSql = "SELECT *\n" + "FROM fuzzy_union.tablea tablea\n" + "UNION ALL\n" + "SELECT *\n"
+        + "FROM fuzzy_union.tablea tablea0";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -57,8 +57,9 @@ public void testNoSchemaEvolutionWithMultipleTables() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql = "" + "SELECT *\n" + "FROM (SELECT *\n" + "FROM fuzzy_union.tablea\n" + "UNION ALL\n"
-        + "SELECT *\n" + "FROM fuzzy_union.tablea)\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablea";
+    String expectedSql =
+        "SELECT *\n" + "FROM (SELECT *\n" + "FROM fuzzy_union.tablea tablea\n" + "UNION ALL\n" + "SELECT *\n"
+            + "FROM fuzzy_union.tablea tablea0) t\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablea tablea1";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -71,8 +72,8 @@ public void testNoSchemaEvolutionWithAlias() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql =
-        "" + "SELECT *\n" + "FROM fuzzy_union.tablea\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablea";
+    String expectedSql = "SELECT *\n" + "FROM fuzzy_union.tablea tablea\n" + "UNION ALL\n" + "SELECT *\n"
+        + "FROM fuzzy_union.tablea tablea0";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -85,8 +86,8 @@ public void testSingleBranchSchemaEvolution() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql = "SELECT *\n" + "FROM fuzzy_union.tableb\n" + "UNION ALL\n"
-        + "SELECT a, generic_project(b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tablec";
+    String expectedSql = "SELECT *\n" + "FROM fuzzy_union.tableb tableb\n" + "UNION ALL\n"
+        + "SELECT tablec.a, generic_project(tablec.b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tablec tablec";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -105,8 +106,8 @@ public void testDoubleBranchSameSchemaEvolution() {
     // This query currently does not have any generic_projections despite the top level view schema being inconsistent
     // because the schemas of the branches evolved the same way.
     // This unit test illustrates this behaviour; however, we can re-evaluate our desired behaviour later on.
-    String expectedSql =
-        "SELECT *\n" + "FROM fuzzy_union.tabled\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablee";
+    String expectedSql = "SELECT *\n" + "FROM fuzzy_union.tabled tabled\n" + "UNION ALL\n" + "SELECT *\n"
+        + "FROM fuzzy_union.tablee tablee";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -119,8 +120,9 @@ public void testDoubleBranchDifferentSchemaEvolution() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql = "SELECT a, generic_project(b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tablef\n"
-        + "UNION ALL\n" + "SELECT a, generic_project(b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tableg";
+    String expectedSql = "SELECT tablef.a, generic_project(tablef.b, 'struct<b1:string>') b\n"
+        + "FROM fuzzy_union.tablef tablef\n" + "UNION ALL\n"
+        + "SELECT tableg.a, generic_project(tableg.b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tableg tableg";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -133,10 +135,11 @@ public void testMoreThanTwoBranchesSchemaEvolution() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql =
-        "SELECT *\n" + "FROM (SELECT a, generic_project(b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tablef\n"
-            + "UNION ALL\n" + "SELECT a, generic_project(b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tableg)\n"
-            + "UNION ALL\n" + "SELECT a, generic_project(b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tablef";
+    String expectedSql = "SELECT *\n" + "FROM (SELECT tablef.a, generic_project(tablef.b, 'struct<b1:string>') b\n"
+        + "FROM fuzzy_union.tablef tablef\n" + "UNION ALL\n"
+        + "SELECT tableg.a, generic_project(tableg.b, 'struct<b1:string>') b\n" + "FROM fuzzy_union.tableg tableg) t1\n"
+        + "UNION ALL\n" + "SELECT tablef0.a, generic_project(tablef0.b, 'struct<b1:string>') b\n"
+        + "FROM fuzzy_union.tablef tablef0";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -149,8 +152,8 @@ public void testMapWithStructValueSchemaEvolution() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql = "SELECT a, generic_project(b, 'map<string,struct<b1:string>>') b\n"
-        + "FROM fuzzy_union.tableh\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablei";
+    String expectedSql = "SELECT tableh.a, generic_project(tableh.b, 'map<string,struct<b1:string>>') b\n"
+        + "FROM fuzzy_union.tableh tableh\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablei tablei";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -163,8 +166,8 @@ public void testArrayWithStructValueSchemaEvolution() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql = "SELECT a, generic_project(b, 'array<struct<b1:string>>') b\n" + "FROM fuzzy_union.tablej\n"
-        + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablek";
+    String expectedSql = "SELECT tablej.a, generic_project(tablej.b, 'array<struct<b1:string>>') b\n"
+        + "FROM fuzzy_union.tablej tablej\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablek tablek";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -178,8 +181,8 @@ public void testDeeplyNestedStructSchemaEvolution() {
     String expandedSql = coralSpark.getSparkSql();
 
     String expectedSql =
-        "SELECT a, generic_project(b, 'struct<b1:string,b2:struct<b3:string,b4:struct<b5:string>>>') b\n"
-            + "FROM fuzzy_union.tablel\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablem";
+        "SELECT tablel.a, generic_project(tablel.b, 'struct<b1:string,b2:struct<b3:string,b4:struct<b5:string>>>') b\n"
+            + "FROM fuzzy_union.tablel tablel\n" + "UNION ALL\n" + "SELECT *\n" + "FROM fuzzy_union.tablem tablem";
 
     assertEquals(expandedSql, expectedSql);
   }
@@ -192,8 +195,9 @@ public void testSameSchemaEvolutionWithDifferentOrdering() {
     CoralSpark coralSpark = CoralSpark.create(relNode);
     String expandedSql = coralSpark.getSparkSql();
 
-    String expectedSql = "SELECT *\n" + "FROM fuzzy_union.tablen\n" + "UNION ALL\n"
-        + "SELECT a, generic_project(b, 'struct<b2:double,b1:string,b0:int>') b\n" + "FROM fuzzy_union.tableo";
+    String expectedSql = "SELECT *\n" + "FROM fuzzy_union.tablen tablen\n" + "UNION ALL\n"
+        + "SELECT tableo.a, generic_project(tableo.b, 'struct<b2:double,b1:string,b0:int>') b\n"
+        + "FROM fuzzy_union.tableo tableo";
 
     assertEquals(expandedSql, expectedSql);
   }

coral-trino/src/main/java/com/linkedin/coral/trino/rel2trino/RelToTrinoConverter.java

@@ -264,7 +264,7 @@ private boolean isTrinoSupportedUnnest(Uncollect uncollect) {
   public Result visit(Correlate e) {
     final Result leftResult = visitChild(0, e.getLeft()).resetAlias(e.getCorrelVariable(), e.getLeft().getRowType());
     parseCorrelTable(e, leftResult);
-    final Result rightResult = visitChild(1, e.getRight());
+    final Result rightResult = visitChild(1, e.getRight()).resetAlias();
     SqlNode rightLateral = rightResult.node;
     if (rightLateral.getKind() != SqlKind.AS) {
       // LATERAL is only needed in Trino if it's not an AS node.
@@ -280,6 +280,27 @@ public Result visit(Correlate e) {
     return result(join, leftResult, rightResult);
   }
 
+  /**
+   * Override this method to avoid the duplicated alias for {@link org.apache.calcite.rel.logical.LogicalValues}.
+   * So that for the input SQL like `SELECT 1`, the translated SQL will be like:
+   *
+   * SELECT 1
+   * FROM (VALUES  (0)) t (ZERO)
+   *
+   * Without this override, the translated SQL contains duplicated alias `t`:
+   *
+   * SELECT 1
+   * FROM (VALUES  (0)) t (ZERO) t
+   *
+   * which is wrong.
+   */
+  @Override
+  public Result visit(Values e) {
+    final Result originalResult = super.visit(e);
+    return new Result(originalResult.node, originalResult.clauses, null, originalResult.neededType,
+        originalResult.aliases);
+  }
+
   @Override
   public Context aliasContext(Map<String, RelDataType> aliases, boolean qualified) {
     // easier to keep inner class for accessing 'aliases' and 'qualified' variables as closure

coral-trino/src/main/java/com/linkedin/coral/trino/rel2trino/TrinoSqlDialect.java

@@ -30,6 +30,15 @@ public boolean supportsCharSet() {
     return false;
   }
 
+  /**
+   * Override this method so that there will be explicit and correct table alias for selected fields, which is necessary for
+   * data type derivation on SqlNodes
+   */
+  @Override
+  public boolean hasImplicitTableAlias() {
+    return false;
+  }
+
   public void unparseOffsetFetch(SqlWriter writer, SqlNode offset, SqlNode fetch) {
     unparseFetchUsingLimit(writer, offset, fetch);
   }