Better query error classification for user errors

extensions-contrib/spectator-histogram/src/main/java/org/apache/druid/spectator/histogram/sql/SpectatorHistogramPercentileSqlAggregator.java

@@ -42,6 +42,7 @@
 import org.apache.druid.sql.calcite.aggregation.Aggregations;
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.rel.InputAccessor;
 import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
@@ -123,7 +124,8 @@ private Aggregation handleSinglePercentile(
       final List<Aggregation> existingAggregations
   )
   {
-    final double percentile = ((Number) RexLiteral.value(percentileArg)).doubleValue();
+    final Object value = RexLiteral.value(percentileArg);
+    final double percentile = DruidSqlParserUtils.getNumericLiteral(value, "SPECTATOR_PERCENTILE", "percentile").doubleValue();
 
     final String histogramName = StringUtils.format("%s:agg", name);
 

extensions-contrib/spectator-histogram/src/test/java/org/apache/druid/spectator/histogram/sql/SpectatorHistogramSqlAggregatorTest.java

@@ -29,6 +29,7 @@
 import org.apache.druid.data.input.impl.StringDimensionSchema;
 import org.apache.druid.data.input.impl.TimeAndDimsParseSpec;
 import org.apache.druid.data.input.impl.TimestampSpec;
+import org.apache.druid.error.DruidException;
 import org.apache.druid.initialization.DruidModule;
 import org.apache.druid.java.util.common.granularity.Granularities;
 import org.apache.druid.query.Druids;
@@ -57,6 +58,7 @@
 import org.apache.druid.sql.calcite.util.SqlTestFramework.StandardComponentSupplier;
 import org.apache.druid.timeline.DataSegment;
 import org.apache.druid.timeline.partition.LinearShardSpec;
+import org.junit.Assert;
 import org.junit.jupiter.api.Test;
 
 import java.util.Collections;
@@ -551,4 +553,21 @@ public void testSpectatorFunctionsOnNullHistogram()
         ImmutableList.of(new Object[]{null, null, null})
     );
   }
+
+  @Test
+  public void testSpectatorPercentileWithStringLiteral()
+  {
+    // verify invalid queries return 400 (user error)
+    final String query = "SELECT SPECTATOR_PERCENTILE(histogram_metric, '99.99') FROM foo";
+
+    try {
+      testQuery(query, ImmutableList.of(), ImmutableList.of());
+      Assert.fail("Expected DruidException but query succeeded");
+    }
+    catch (DruidException e) {
+      Assert.assertEquals(DruidException.Persona.USER, e.getTargetPersona());
+      Assert.assertEquals(DruidException.Category.INVALID_INPUT, e.getCategory());
+      Assert.assertTrue(e.getMessage().contains("must be a numeric literal"));
+    }
+  }
 }

extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchBaseSqlAggregator.java

@@ -40,6 +40,7 @@
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerConfig;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
@@ -92,7 +93,7 @@ public Aggregation toDruidAggregation(
         return null;
       }
 
-      logK = ((Number) RexLiteral.value(logKarg)).intValue();
+      logK = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(logKarg), "APPROX_COUNT_DISTINCT_DS_HLL", "logK").intValue();
     } else {
       logK = HllSketchAggregatorFactory.DEFAULT_LG_K;
     }

extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchApproxQuantileSqlAggregator.java

@@ -40,6 +40,7 @@
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.rel.InputAccessor;
 import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
@@ -98,7 +99,7 @@ public Aggregation toDruidAggregation(
       return null;
     }
 
-    final float probability = ((Number) RexLiteral.value(probabilityArg)).floatValue();
+    final float probability = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(probabilityArg), "APPROX_QUANTILE_DS", "probability").floatValue();
     final int k;
 
     if (aggregateCall.getArgList().size() >= 3) {
@@ -109,7 +110,7 @@ public Aggregation toDruidAggregation(
         return null;
       }
 
-      k = ((Number) RexLiteral.value(resolutionArg)).intValue();
+      k = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(resolutionArg), "APPROX_QUANTILE_DS", "resolution").intValue();
     } else {
       k = DoublesSketchAggregatorFactory.DEFAULT_K;
     }

extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchBaseSqlAggregator.java

@@ -38,6 +38,7 @@
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerConfig;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
@@ -90,7 +91,7 @@ public Aggregation toDruidAggregation(
         return null;
       }
 
-      sketchSize = ((Number) RexLiteral.value(sketchSizeArg)).intValue();
+      sketchSize = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(sketchSizeArg), "APPROX_COUNT_DISTINCT_DS_THETA", "size").intValue();
     } else {
       sketchSize = SketchAggregatorFactory.DEFAULT_MAX_SKETCH_SIZE;
     }

extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java

@@ -22,6 +22,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import org.apache.druid.error.DruidException;
 import org.apache.druid.initialization.DruidModule;
 import org.apache.druid.java.util.common.Intervals;
 import org.apache.druid.java.util.common.granularity.Granularities;
@@ -71,6 +72,7 @@
 import org.apache.druid.sql.calcite.util.TestDataBuilder;
 import org.apache.druid.timeline.DataSegment;
 import org.apache.druid.timeline.partition.LinearShardSpec;
+import org.junit.Assert;
 import org.junit.jupiter.api.Test;
 
 import java.util.Collections;
@@ -1111,6 +1113,40 @@ public void testSuccessWithSmallMaxStreamLength()
     );
   }
 
+  @Test
+  public void testApproxQuantileWithStringLiteral()
+  {
+    // verify invalid queries return 400 (user error)
+    final String query = "SELECT APPROX_QUANTILE_DS(m1, '0.99') FROM foo";
+
+    try {
+      testQuery(query, ImmutableList.of(), ImmutableList.of());
+      Assert.fail("Expected DruidException but query succeeded");
+    }
+    catch (DruidException e) {
+      Assert.assertEquals(DruidException.Persona.USER, e.getTargetPersona());
+      Assert.assertEquals(DruidException.Category.INVALID_INPUT, e.getCategory());
+      Assert.assertTrue(e.getMessage().contains("Cannot apply 'APPROX_QUANTILE_DS'"));
+    }
+  }
+
+  @Test
+  public void testApproxQuantileWithStringResolution()
+  {
+    // verify invalid queries return 400 (user error)
+    final String query = "SELECT APPROX_QUANTILE_DS(m1, 0.99, '128') FROM foo";
+
+    try {
+      testQuery(query, ImmutableList.of(), ImmutableList.of());
+      Assert.fail("Expected DruidException but query succeeded");
+    }
+    catch (DruidException e) {
+      Assert.assertEquals(DruidException.Persona.USER, e.getTargetPersona());
+      Assert.assertEquals(DruidException.Category.INVALID_INPUT, e.getCategory());
+      Assert.assertTrue(e.getMessage().contains("Cannot apply 'APPROX_QUANTILE_DS'"));
+    }
+  }
+
   private static PostAggregator makeFieldAccessPostAgg(String name)
   {
     return new FieldAccessPostAggregator(name, name);

extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/sql/BloomFilterSqlAggregator.java

@@ -39,6 +39,7 @@
 import org.apache.druid.sql.calcite.expression.DefaultOperandTypeChecker;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.rel.InputAccessor;
@@ -88,7 +89,7 @@ public Aggregation toDruidAggregation(
       return null;
     }
 
-    final int maxNumEntries = ((Number) RexLiteral.value(maxNumEntriesOperand)).intValue();
+    final int maxNumEntries = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(maxNumEntriesOperand), "BLOOM_FILTER", "maxNumEntries").intValue();
 
     // Look for existing matching aggregatorFactory.
     for (final Aggregation existing : existingAggregations) {

extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregator.java

@@ -43,6 +43,7 @@
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DefaultOperandTypeChecker;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.rel.InputAccessor;
 import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
@@ -91,7 +92,7 @@ public Aggregation toDruidAggregation(
       return null;
     }
 
-    final float probability = ((Number) RexLiteral.value(probabilityArg)).floatValue();
+    final float probability = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(probabilityArg), "APPROX_QUANTILE", "probability").floatValue();
     final int resolution;
 
     if (aggregateCall.getArgList().size() >= 3) {
@@ -102,7 +103,7 @@ public Aggregation toDruidAggregation(
         return null;
       }
 
-      resolution = ((Number) RexLiteral.value(resolutionArg)).intValue();
+      resolution = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(resolutionArg), "APPROX_QUANTILE", "resolution").intValue();
     } else {
       resolution = ApproximateHistogram.DEFAULT_HISTOGRAM_SIZE;
     }

sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/ArrayConcatSqlAggregator.java

@@ -43,6 +43,7 @@
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.rel.InputAccessor;
@@ -83,7 +84,7 @@ public Aggregation toDruidAggregation(
         // maxBytes must be a literal
         return null;
       }
-      maxSizeBytes = ((Number) RexLiteral.value(maxBytes)).intValue();
+      maxSizeBytes = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(maxBytes), "ARRAY_CONCAT_AGG", "maxBytes").intValue();
     }
     final DruidExpression arg = Expressions.toDruidExpression(plannerContext, inputAccessor.getInputRowSignature(), arguments.get(0));
     final ExprMacroTable macroTable = plannerContext.getPlannerToolbox().exprMacroTable();

sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/ArraySqlAggregator.java

@@ -45,6 +45,7 @@
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.rel.InputAccessor;
@@ -86,7 +87,7 @@ public Aggregation toDruidAggregation(
         // maxBytes must be a literal
         return null;
       }
-      maxSizeBytes = ((Number) RexLiteral.value(maxBytes)).intValue();
+      maxSizeBytes = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(maxBytes), "ARRAY_AGG", "maxBytes").intValue();
     }
     final DruidExpression arg = Expressions.toDruidExpression(plannerContext, inputAccessor.getInputRowSignature(), arguments.get(0));
     if (arg == null) {

sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/StringSqlAggregator.java

@@ -50,6 +50,7 @@
 import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
 import org.apache.druid.sql.calcite.planner.Calcites;
 import org.apache.druid.sql.calcite.planner.PlannerContext;
 import org.apache.druid.sql.calcite.rel.InputAccessor;
@@ -130,7 +131,7 @@ public Aggregation toDruidAggregation(
         // maxBytes must be a literal
         return null;
       }
-      maxSizeBytes = ((Number) RexLiteral.value(maxBytes)).intValue();
+      maxSizeBytes = DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(maxBytes), "STRING_AGG", "maxBytes").intValue();
     }
 
     final DruidExpression arg = arguments.get(0);

sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java

@@ -665,4 +665,51 @@ public static DruidException problemParsing(String message)
   {
     return InvalidSqlInput.exception(message);
   }
+
+  /**
+   * Creates a DruidException for invalid SQL function parameter types.
+   *
+   * @param functionName the SQL function name (e.g., "SPECTATOR_PERCENTILE")
+   * @param parameterName the parameter name
+   * @param expectedType the expected type
+   * @param actualValue the value provided needed to determine type
+   * @return DruidException with INVALID_INPUT category and USER persona
+   */                                                                                                                                                   
+  public static DruidException invalidParameterTypeException(
+      String functionName,
+      String parameterName,
+      String expectedType,
+      @Nullable Object actualValue
+  )                                                                                                                                                     
+  {
+    final String actualType = actualValue == null ? "NULL" : actualValue.getClass().getSimpleName();
+    return InvalidSqlInput.exception(
+        "%s parameter `%s` must be a %s literal, got %s",
+        functionName,
+        parameterName,
+        expectedType,
+        actualType
+    );
+  }                                                                                                                                                     
+
+  /**
+   * Validates and returns a numeric value from a RexLiteral, or throws invalidParameterTypeException if invalid.
+   *
+   * @param value the value extracted from RexLiteral.value()
+   * @param functionName the SQL function name
+   * @param parameterName the parameter name
+   * @return the value as a Number
+   * @throws DruidException if value is not a Number
+   */                                                                                                                                                   
+  public static Number getNumericLiteral(
+      @Nullable Object value,
+      String functionName,
+      String parameterName
+  )
+  {
+    if (!(value instanceof Number)) {
+      throw invalidParameterTypeException(functionName, parameterName, "numeric", value);
+    }
+    return (Number) value;
+  }
 }

sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java

@@ -14377,6 +14377,57 @@ public void testStringAggMaxBytes()
     );
   }
 
+  @Test
+  public void testStringAggWithStringMaxBytes()
+  {
+    // verify invalid queries return 400 (user error)
+    final String query = "SELECT STRING_AGG(dim1, ',', 'abc') FROM foo";
+
+    try {
+      testQuery(query, ImmutableList.of(), ImmutableList.of());
+      Assert.fail("Expected DruidException but query succeeded");
+    }
+    catch (DruidException e) {
+      Assert.assertEquals(DruidException.Persona.USER, e.getTargetPersona());
+      Assert.assertEquals(DruidException.Category.INVALID_INPUT, e.getCategory());
+      Assert.assertTrue(e.getMessage().contains("parameter `maxBytes` must be a numeric literal"));
+    }
+  }
+
+  @Test
+  public void testArrayAggWithStringMaxBytes()
+  {
+    // verify invalid queries return 400 (user error)
+    final String query = "SELECT ARRAY_AGG(dim1, 'abc') FROM foo";
+
+    try {
+      testQuery(query, ImmutableList.of(), ImmutableList.of());
+      Assert.fail("Expected DruidException but query succeeded");
+    }
+    catch (DruidException e) {
+      Assert.assertEquals(DruidException.Persona.USER, e.getTargetPersona());
+      Assert.assertEquals(DruidException.Category.INVALID_INPUT, e.getCategory());
+      Assert.assertTrue(e.getMessage().contains("parameter `maxBytes` must be a numeric literal"));
+    }
+  }
+
+  @Test
+  public void testArrayConcatAggWithStringMaxBytes()
+  {
+    // verify invalid queries return 400 (user error)
+    final String query = "SELECT ARRAY_CONCAT_AGG(MV_TO_ARRAY(dim3), 'abc') FROM foo";
+
+    try {
+      testQuery(query, ImmutableList.of(), ImmutableList.of());
+      Assert.fail("Expected DruidException but query succeeded");
+    }
+    catch (DruidException e) {
+      Assert.assertEquals(DruidException.Persona.USER, e.getTargetPersona());
+      Assert.assertEquals(DruidException.Category.INVALID_INPUT, e.getCategory());
+      Assert.assertTrue(e.getMessage().contains("parameter `maxBytes` must be a numeric literal"));
+    }
+  }
+
   /**
    * see {@link TestDataBuilder#RAW_ROWS1_WITH_NUMERIC_DIMS}
    * for the input data source of this test