You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2023/03/08 00:08:56 UTC

[impala] branch master updated: IMPALA-9551: (Addendum) disable sorting if select list contains struct containing collection

This is an automated email from the ASF dual-hosted git repository.

stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new a28da34a2 IMPALA-9551: (Addendum) disable sorting if select list contains struct containing collection
a28da34a2 is described below

commit a28da34a216497af9fdc01a56cdf4056fbbe1530
Author: Daniel Becker <da...@cloudera.com>
AuthorDate: Tue Mar 7 14:24:36 2023 +0100

    IMPALA-9551: (Addendum) disable sorting if select list contains struct containing collection
    
    Sorting is not supported if the select list contains collection columns
    (see IMPALA-10939). IMPALA-9551 added support for mixed complex types
    (collections in structs and structs in collections). However, the case
    of having structs containing collections in the select list when sorting
    was not handled explicitly. The query
    
      select id, struct_contains_arr from collection_struct_mix order by id;
    
    resulted in
      ERROR: IllegalStateException: null
    
    After this change, a meaningful error message is given (the same as in
    the case of pure collection columns):
    
      ERROR: IllegalStateException: Sorting is not supported if the select
      list contains collection columns.
    
    The check for collections in the sorting tuple was moved to an earlier
    stage of analysis from SingleNodePlanner to QueryStmt, as otherwise we
    would hit another precondition check first in the case of structs
    containing collections.
    
    Testing:
     - Added tests in mixed-collections-and-structs.test that test sorting
       when a struct in the select list contains an array and a map
       respectively.
    
    Change-Id: I09ac27cba34ee7c6325a7e7895f3a3c9e1a088e5
    Reviewed-on: http://gerrit.cloudera.org:8080/19597
    Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../main/java/org/apache/impala/analysis/QueryStmt.java  |  8 ++++++++
 fe/src/main/java/org/apache/impala/catalog/Type.java     | 16 ++++++++++++++++
 .../org/apache/impala/planner/SingleNodePlanner.java     |  4 ----
 .../queries/QueryTest/mixed-collections-and-structs.test | 14 ++++++++++++++
 4 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
index e7f7f6385..1893f3021 100644
--- a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
@@ -24,6 +24,8 @@ import java.util.Set;
 
 import com.google.common.collect.Sets;
 import org.apache.impala.catalog.FeView;
+import org.apache.impala.catalog.StructField;
+import org.apache.impala.catalog.StructType;
 import org.apache.impala.catalog.Type;
 import org.apache.impala.catalog.View;
 import org.apache.impala.common.AnalysisException;
@@ -319,6 +321,12 @@ public abstract class QueryStmt extends StatementBase {
             orderingExpr.getType().toSql()));
       }
     }
+
+    for (Expr expr: resultExprs_) {
+      Preconditions.checkState(!expr.getType().containsCollection(),
+          "Sorting is not supported if the select list contains collection columns.");
+    }
+
     sortInfo_.createSortTupleInfo(resultExprs_, analyzer);
 
     ExprSubstitutionMap smap = sortInfo_.getOutputSmap();
diff --git a/fe/src/main/java/org/apache/impala/catalog/Type.java b/fe/src/main/java/org/apache/impala/catalog/Type.java
index 7e4c2fb6f..ac418714e 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Type.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Type.java
@@ -240,6 +240,22 @@ public abstract class Type {
     return this instanceof CollectionStructType;
   }
 
+  /**
+   * Returns true if this type
+   *  - is a collection type or
+   *  - contains a collection type (recursively).
+   */
+  public boolean containsCollection() {
+    if (isCollectionType()) return true;
+    if (isStructType()) {
+      for (StructField field : ((StructType) this).getFields()) {
+        Type fieldType = field.getType();
+        if (fieldType.containsCollection()) return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * Returns true if Impala supports this type in the metdata. It does not mean we
    * can manipulate data of this type. For tables that contain columns with these
diff --git a/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java b/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
index 197f8731f..ec2a61617 100644
--- a/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
+++ b/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
@@ -318,10 +318,6 @@ public class SingleNodePlanner {
     }
 
     if (stmt.evaluateOrderBy() && sortHasMaterializedSlots) {
-      for (Expr expr: stmt.getResultExprs()) {
-        Preconditions.checkState(!expr.getType().isCollectionType(),
-            "Sorting is not supported if the select list contains collection columns.");
-      }
       root = createSortNode(ctx_, analyzer, root, stmt.getSortInfo(), stmt.getLimit(),
           stmt.getOffset(), stmt.hasLimit(), disableTopN);
     } else {
diff --git a/testdata/workloads/functional-query/queries/QueryTest/mixed-collections-and-structs.test b/testdata/workloads/functional-query/queries/QueryTest/mixed-collections-and-structs.test
index 56ba1c5d1..ff6c50011 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/mixed-collections-and-structs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/mixed-collections-and-structs.test
@@ -266,6 +266,20 @@ select id, a.item, a.item.inner_struct, a.item.small from sub,
 INT,STRING,STRING,SMALLINT
 ====
 ---- QUERY
+# Sorting is not supported yet for collections: IMPALA-10939. Test with a struct that
+# contains an array.
+select id, struct_contains_arr from collection_struct_mix order by id
+---- CATCH
+IllegalStateException: Sorting is not supported if the select list contains collection columns.
+====
+---- QUERY
+# Sorting is not supported yet for collections: IMPALA-10939. Test with a struct that
+# contains a map.
+select id, struct_contains_map from collection_struct_mix order by id;
+---- CATCH
+IllegalStateException: Sorting is not supported if the select list contains collection columns.
+====
+---- QUERY
 # Zipping unnest an array that contains a struct.
 select unnest(arr_contains_struct) from collection_struct_mix;
 ---- CATCH