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

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


The following commit(s) were added to refs/heads/master by this push:
     new 1871a1a  ARRAY_AGG and STRING_AGG will through errors if invoked on a 
complex datatype (#12089)
1871a1a is described below

commit 1871a1ab18067c3db1e65e52538a861af2f5800d
Author: somu-imply <93540295+somu-im...@users.noreply.github.com>
AuthorDate: Tue Dec 21 17:41:04 2021 -0800

    ARRAY_AGG and STRING_AGG will through errors if invoked on a complex 
datatype (#12089)
---
 .../aggregation/builtin/ArraySqlAggregator.java    |  4 +++
 .../aggregation/builtin/StringSqlAggregator.java   | 27 ++++++++++++++++--
 .../apache/druid/sql/calcite/CalciteQueryTest.java | 33 ++++++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git 
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/ArraySqlAggregator.java
 
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/ArraySqlAggregator.java
index 56f1fe8..e2213a3 100644
--- 
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/ArraySqlAggregator.java
+++ 
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/ArraySqlAggregator.java
@@ -51,6 +51,7 @@ import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.planner.UnsupportedSQLQueryException;
 import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
+import org.apache.druid.sql.calcite.table.RowSignatures;
 
 import javax.annotation.Nullable;
 
@@ -187,6 +188,9 @@ public class ArraySqlAggregator implements SqlAggregator
       if (SqlTypeUtil.isArray(type)) {
         throw new UnsupportedSQLQueryException("Cannot use ARRAY_AGG on array 
inputs %s", type);
       }
+      if (type instanceof RowSignatures.ComplexSqlType) {
+        throw new UnsupportedSQLQueryException("Cannot use ARRAY_AGG on 
complex inputs %s", type);
+      }
       return Calcites.createSqlArrayTypeWithNullability(
           sqlOperatorBinding.getTypeFactory(),
           type.getSqlTypeName(),
diff --git 
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/StringSqlAggregator.java
 
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/StringSqlAggregator.java
index c11f5a0..72bfe9e 100644
--- 
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/StringSqlAggregator.java
+++ 
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/StringSqlAggregator.java
@@ -22,14 +22,17 @@ package org.apache.druid.sql.calcite.aggregation.builtin;
 import com.google.common.collect.ImmutableSet;
 import org.apache.calcite.rel.core.AggregateCall;
 import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexLiteral;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.SqlFunctionCategory;
 import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperatorBinding;
 import org.apache.calcite.sql.type.InferTypes;
 import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.SqlReturnTypeInference;
 import org.apache.calcite.sql.type.SqlTypeFamily;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.util.Optionality;
@@ -49,7 +52,9 @@ import 
org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.Expressions;
 import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.planner.UnsupportedSQLQueryException;
 import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
+import org.apache.druid.sql.calcite.table.RowSignatures;
 
 import javax.annotation.Nullable;
 import java.util.List;
@@ -181,16 +186,34 @@ public class StringSqlAggregator implements SqlAggregator
     }
   }
 
+  static class StringAggReturnTypeInference implements SqlReturnTypeInference
+  {
+    @Override
+    public RelDataType inferReturnType(SqlOperatorBinding sqlOperatorBinding)
+    {
+      RelDataType type = sqlOperatorBinding.getOperandType(0);
+      if (type instanceof RowSignatures.ComplexSqlType) {
+        throw new UnsupportedSQLQueryException("Cannot use STRING_AGG on 
complex inputs %s", type);
+      }
+      return Calcites.createSqlTypeWithNullability(
+          sqlOperatorBinding.getTypeFactory(),
+          SqlTypeName.VARCHAR,
+          true
+      );
+    }
+  }
+
   private static class StringAggFunction extends SqlAggFunction
   {
+    private static final StringAggReturnTypeInference RETURN_TYPE_INFERENCE = 
new StringAggReturnTypeInference();
+
     StringAggFunction()
     {
       super(
           NAME,
           null,
           SqlKind.OTHER_FUNCTION,
-          opBinding ->
-              
Calcites.createSqlTypeWithNullability(opBinding.getTypeFactory(), 
SqlTypeName.VARCHAR, true),
+          RETURN_TYPE_INFERENCE,
           InferTypes.ANY_NULLABLE,
           OperandTypes.or(
               OperandTypes.and(
diff --git 
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java 
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 90b2912..01a994b 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -5249,6 +5249,39 @@ public class CalciteQueryTest extends 
BaseCalciteQueryTest
   }
 
   @Test
+  public void testArrayAggQueryOnComplexDatatypes() throws Exception
+  {
+    try {
+      testQuery("SELECT ARRAY_AGG(unique_dim1) FROM druid.foo", 
ImmutableList.of(), ImmutableList.of());
+      Assert.fail("query execution should fail");
+    }
+    catch (SqlPlanningException e) {
+      Assert.assertTrue(
+          e.getMessage().contains("Cannot use ARRAY_AGG on complex inputs 
COMPLEX<hyperUnique>")
+      );
+      Assert.assertEquals(PlanningError.VALIDATION_ERROR.getErrorCode(), 
e.getErrorCode());
+      Assert.assertEquals(PlanningError.VALIDATION_ERROR.getErrorClass(), 
e.getErrorClass());
+    }
+  }
+
+  @Test
+  public void testStringAggQueryOnComplexDatatypes() throws Exception
+  {
+    try {
+      testQuery("SELECT STRING_AGG(unique_dim1, ',') FROM druid.foo", 
ImmutableList.of(), ImmutableList.of());
+      Assert.fail("query execution should fail");
+    }
+    catch (SqlPlanningException e) {
+      Assert.assertTrue(
+          e.getMessage().contains("Cannot use STRING_AGG on complex inputs 
COMPLEX<hyperUnique>")
+      );
+      Assert.assertEquals(PlanningError.VALIDATION_ERROR.getErrorCode(), 
e.getErrorCode());
+      Assert.assertEquals(PlanningError.VALIDATION_ERROR.getErrorClass(), 
e.getErrorClass());
+    }
+  }
+
+
+  @Test
   public void testCountStarWithBoundFilterSimplifyAnd() throws Exception
   {
     testQuery(

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org

Reply via email to