[fix](nereids)fix bug of duplicate name of inline view (apache#25627)

HappenLee · Oct 20, 2023 · a11cde7 · a11cde7
1 parent 42e5a33
commit a11cde7
Show file tree

Hide file tree

Showing 7 changed files with 153 additions and 16 deletions.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundSlot.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundSlot.java
@@ -63,6 +63,11 @@ public List<String> getQualifier() {
         return nameParts.subList(0, nameParts.size() - 1);
     }
 
+    @Override
+    public String getInternalName() {
+        return getName();
+    }
+
     @Override
     public String toSql() {
         return nameParts.stream().map(Utils::quoteIfNeeded).reduce((left, right) -> left + "." + right).orElse("");

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
@@ -709,11 +709,11 @@ private LogicalTVFRelation bindTableValuedFunction(UnboundTVFRelation unboundTVF
     private void checkSameNameSlot(List<Slot> childOutputs, String subQueryAlias) {
         Set<String> nameSlots = new HashSet<>();
         for (Slot s : childOutputs) {
-            if (nameSlots.contains(s.getName())) {
+            if (nameSlots.contains(s.getInternalName())) {
                 throw new AnalysisException("Duplicated inline view column alias: '" + s.getName()
                         + "'" + " in inline view: '" + subQueryAlias + "'");
             } else {
-                nameSlots.add(s.getName());
+                nameSlots.add(s.getInternalName());
             }
         }
     }

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Alias.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Alias.java
@@ -27,6 +27,7 @@
 
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 
 /**
  * Expression for alias, such as col1 as c1.
@@ -73,7 +74,8 @@ public Slot toSlot() throws UnboundException {
         return new SlotReference(exprId, name, child().getDataType(), child().nullable(), qualifier,
                 child() instanceof SlotReference
                         ? ((SlotReference) child()).getColumn().orElse(null)
-                        : null);
+                        : null,
+                nameFromChild ? Optional.of(child().toString()) : Optional.of(name));
     }
 
     @Override

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/ArrayItemReference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/ArrayItemReference.java
@@ -28,6 +28,7 @@
 
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 
 /**
  * it is item from array, which used in lambda function
@@ -138,7 +139,7 @@ static class ArrayItemSlot extends SlotReference implements SlotNotFromChildren
          * @param nullable true if nullable
          */
         public ArrayItemSlot(ExprId exprId, String name, DataType dataType, boolean nullable) {
-            super(exprId, name, dataType, nullable, ImmutableList.of(), null);
+            super(exprId, name, dataType, nullable, ImmutableList.of(), null, Optional.empty());
         }
 
         @Override

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Slot.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Slot.java
@@ -52,4 +52,8 @@ public Slot withName(String name) {
     public Slot withExprId(ExprId exprId) {
         throw new RuntimeException("Do not implement");
     }
+
+    public String getInternalName() {
+        throw new RuntimeException("Do not implement");
+    }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/SlotReference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/SlotReference.java
@@ -38,22 +38,34 @@ public class SlotReference extends Slot {
     protected final DataType dataType;
     protected final boolean nullable;
     protected final List<String> qualifier;
+
+    // the unique string representation of a SlotReference
+    // different SlotReference will have different internalName
+    // TODO: remove this member variable after mv selection is refactored
+    protected final Optional<String> internalName;
+
     private final Column column;
 
     public SlotReference(String name, DataType dataType) {
-        this(StatementScopeIdGenerator.newExprId(), name, dataType, true, ImmutableList.of(), null);
+        this(StatementScopeIdGenerator.newExprId(), name, dataType, true, ImmutableList.of(), null, Optional.empty());
     }
 
     public SlotReference(String name, DataType dataType, boolean nullable) {
-        this(StatementScopeIdGenerator.newExprId(), name, dataType, nullable, ImmutableList.of(), null);
+        this(StatementScopeIdGenerator.newExprId(), name, dataType, nullable, ImmutableList.of(),
+                null, Optional.empty());
     }
 
     public SlotReference(String name, DataType dataType, boolean nullable, List<String> qualifier) {
-        this(StatementScopeIdGenerator.newExprId(), name, dataType, nullable, qualifier, null);
+        this(StatementScopeIdGenerator.newExprId(), name, dataType, nullable, qualifier, null, Optional.empty());
     }
 
     public SlotReference(ExprId exprId, String name, DataType dataType, boolean nullable, List<String> qualifier) {
-        this(exprId, name, dataType, nullable, qualifier, null);
+        this(exprId, name, dataType, nullable, qualifier, null, Optional.empty());
+    }
+
+    public SlotReference(ExprId exprId, String name, DataType dataType, boolean nullable,
+                         List<String> qualifier, @Nullable Column column) {
+        this(exprId, name, dataType, nullable, qualifier, column, Optional.empty());
     }
 
     /**
@@ -65,15 +77,17 @@ public SlotReference(ExprId exprId, String name, DataType dataType, boolean null
      * @param nullable true if nullable
      * @param qualifier slot reference qualifier
      * @param column the column which this slot come from
+     * @param internalName the internalName of this slot
      */
     public SlotReference(ExprId exprId, String name, DataType dataType, boolean nullable,
-            List<String> qualifier, @Nullable Column column) {
+                         List<String> qualifier, @Nullable Column column, Optional<String> internalName) {
         this.exprId = exprId;
         this.name = name;
         this.dataType = dataType;
         this.qualifier = ImmutableList.copyOf(Objects.requireNonNull(qualifier, "qualifier can not be null"));
         this.nullable = nullable;
         this.column = column;
+        this.internalName = internalName.isPresent() ? internalName : Optional.of(name);
     }
 
     public static SlotReference of(String name, DataType type) {
@@ -83,13 +97,13 @@ public static SlotReference of(String name, DataType type) {
     public static SlotReference fromColumn(Column column, List<String> qualifier) {
         DataType dataType = DataType.fromCatalogType(column.getType());
         return new SlotReference(StatementScopeIdGenerator.newExprId(), column.getName(), dataType,
-                column.isAllowNull(), qualifier, column);
+                column.isAllowNull(), qualifier, column, Optional.empty());
     }
 
     public static SlotReference fromColumn(Column column, String name, List<String> qualifier) {
         DataType dataType = DataType.fromCatalogType(column.getType());
         return new SlotReference(StatementScopeIdGenerator.newExprId(), name, dataType,
-            column.isAllowNull(), qualifier, column);
+            column.isAllowNull(), qualifier, column, Optional.empty());
     }
 
     @Override
@@ -117,6 +131,11 @@ public boolean nullable() {
         return nullable;
     }
 
+    @Override
+    public String getInternalName() {
+        return internalName.get();
+    }
+
     @Override
     public String toSql() {
         return name;
@@ -185,22 +204,22 @@ public SlotReference withNullable(boolean newNullable) {
         if (this.nullable == newNullable) {
             return this;
         }
-        return new SlotReference(exprId, name, dataType, newNullable, qualifier, column);
+        return new SlotReference(exprId, name, dataType, newNullable, qualifier, column, internalName);
     }
 
     @Override
     public SlotReference withQualifier(List<String> qualifier) {
-        return new SlotReference(exprId, name, dataType, nullable, qualifier, column);
+        return new SlotReference(exprId, name, dataType, nullable, qualifier, column, internalName);
     }
 
     @Override
     public SlotReference withName(String name) {
-        return new SlotReference(exprId, name, dataType, nullable, qualifier, column);
+        return new SlotReference(exprId, name, dataType, nullable, qualifier, column, internalName);
     }
 
     @Override
     public SlotReference withExprId(ExprId exprId) {
-        return new SlotReference(exprId, name, dataType, nullable, qualifier, column);
+        return new SlotReference(exprId, name, dataType, nullable, qualifier, column, internalName);
     }
 
     public boolean isVisible() {

diff --git a/regression-test/suites/nereids_p0/subquery/test_duplicate_name_in_view.groovy b/regression-test/suites/nereids_p0/subquery/test_duplicate_name_in_view.groovy
@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-suite("inlineview_with_project") {
+suite("test_duplicate_name_in_view") {
     sql "SET enable_nereids_planner=true"
     sql "SET enable_fallback_to_original_planner=false"
     sql """
@@ -58,6 +58,112 @@ suite("inlineview_with_project") {
 
     }
 
+    test {
+        sql """
+             SELECT *
+                FROM 
+                    (SELECT issue_19611_t1.c0 ,
+                        issue_19611_t1.c0
+                    FROM issue_19611_t1 ) tmp;
+        """
+        exception "Duplicated inline view column alias: 'c0' in inline view: 'tmp'"
+    }
+
+    test {
+        sql """
+             SELECT *
+                FROM 
+                    (SELECT issue_19611_t1.c0 + 1,
+                        issue_19611_t1.c0 + 1
+                    FROM issue_19611_t1 ) tmp;
+        """
+        exception "Duplicated inline view column alias: '(c0 + 1)' in inline view: 'tmp'"
+    }
+
+    test {
+        sql """
+             SELECT *
+                FROM 
+                    (SELECT issue_19611_t1.c0 ,
+                        issue_19611_t0.c0
+                    FROM issue_19611_t1, issue_19611_t0 ) tmp;
+        """
+        exception "Duplicated inline view column alias: 'c0' in inline view: 'tmp'"
+    }
+
+    test {
+        sql """
+             SELECT *
+                FROM 
+                    (SELECT issue_19611_t1.c0 a,
+                        issue_19611_t1.c0 a
+                    FROM issue_19611_t1 ) tmp;
+        """
+        exception "Duplicated inline view column alias: 'a' in inline view: 'tmp'"
+    }
+
+    test {
+        sql """
+             SELECT *
+                FROM 
+                    (SELECT issue_19611_t0.c0 + 1 a,
+                        issue_19611_t1.c0 + 1 a
+                    FROM issue_19611_t1, issue_19611_t0 ) tmp;
+        """
+        exception "Duplicated inline view column alias: 'a' in inline view: 'tmp'"
+    }
+
+    test {
+        sql """
+             SELECT *
+                FROM 
+                    (SELECT '2023-10-07', '2023-10-07'
+                    FROM issue_19611_t1 ) tmp;
+        """
+        exception "Duplicated inline view column alias: ''2023-10-07'' in inline view: 'tmp'"
+    }
+
+    test {
+        sql """
+             SELECT *
+                FROM 
+                    (SELECT '2023-10-07' a, '2023-10-07' a
+                    FROM issue_19611_t1 ) tmp;
+        """
+        exception "Duplicated inline view column alias: 'a' in inline view: 'tmp'"
+    }
+
+    sql """SELECT *
+            FROM 
+                (SELECT issue_19611_t1.c0 + 1,
+                    issue_19611_t0.c0 + 1
+                FROM issue_19611_t1, issue_19611_t0 ) tmp;
+    """
+
+    sql """SELECT *
+            FROM 
+                (SELECT issue_19611_t1.c0 a,
+                    issue_19611_t1.c0 b
+                FROM issue_19611_t1 ) tmp;"""
+
+    sql """SELECT *
+            FROM 
+                (SELECT issue_19611_t1.c0 a,
+                    issue_19611_t1.c0
+                FROM issue_19611_t1 ) tmp;"""
+
+    sql """SELECT *
+            FROM 
+                (SELECT '2023-10-07' a, '2023-10-07' b
+                FROM issue_19611_t1 ) tmp;
+    """
+
+    sql """SELECT *
+            FROM 
+                (SELECT '2023-10-07' a, '2023-10-07'
+                FROM issue_19611_t1 ) tmp;
+    """
+
 
     sql """
         drop table if exists issue_19611_t0;