[SPARK-33951][SQL] Distinguish the error between filter and distinct

### What changes were proposed in this pull request?
The error messages for specifying filter and distinct for the aggregate function are mixed together and should be separated. This can increase readability and ease of use.

### Why are the changes needed?
increase readability and ease of use.

### Does this PR introduce _any_ user-facing change?
'Yes'.

### How was this patch tested?
Jenkins test

Closes #30982 from beliefer/SPARK-33951.

Lead-authored-by: gengjiaan <gengjiaan@360.cn>
Co-authored-by: beliefer <beliefer@163.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
This commit is contained in:
gengjiaan 2021-01-04 05:44:00 +00:00 committed by Wenchen Fan
parent 67195d0d97
commit b037930952
4 changed files with 35 additions and 30 deletions

View file

@ -263,13 +263,8 @@ object QueryCompilationErrors {
s"its class is $classCanonicalName, which is not a generator.") s"its class is $classCanonicalName, which is not a generator.")
} }
def distinctOrFilterOnlyWithAggregateFunctionError(prettyName: String): Throwable = { def functionWithUnsupportedSyntaxError(prettyName: String, syntax: String): Throwable = {
new AnalysisException("DISTINCT or FILTER specified, " + new AnalysisException(s"Function $prettyName does not support $syntax")
s"but $prettyName is not an aggregate function")
}
def ignoreNullsWithUnsupportedFunctionError(prettyName: String): Throwable = {
new AnalysisException(s"Function $prettyName does not support IGNORE NULLS")
} }
def nonDeterministicFilterInAggregateError(): Throwable = { def nonDeterministicFilterInAggregateError(): Throwable = {

View file

@ -2120,24 +2120,30 @@ class Analyzer(override val catalogManager: CatalogManager)
// the context of a Window clause. They do not need to be wrapped in an // the context of a Window clause. They do not need to be wrapped in an
// AggregateExpression. // AggregateExpression.
case wf: AggregateWindowFunction => case wf: AggregateWindowFunction =>
if (isDistinct || filter.isDefined) { if (isDistinct) {
throw QueryCompilationErrors.distinctOrFilterOnlyWithAggregateFunctionError( throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
wf.prettyName) wf.prettyName, "DISTINCT")
} else if (filter.isDefined) {
throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
wf.prettyName, "FILTER clause")
} else if (ignoreNulls) { } else if (ignoreNulls) {
wf match { wf match {
case nthValue: NthValue => case nthValue: NthValue =>
nthValue.copy(ignoreNulls = ignoreNulls) nthValue.copy(ignoreNulls = ignoreNulls)
case _ => case _ =>
throw QueryCompilationErrors.ignoreNullsWithUnsupportedFunctionError( throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
wf.prettyName) wf.prettyName, "IGNORE NULLS")
} }
} else { } else {
wf wf
} }
case owf: FrameLessOffsetWindowFunction => case owf: FrameLessOffsetWindowFunction =>
if (isDistinct || filter.isDefined) { if (isDistinct) {
throw QueryCompilationErrors.distinctOrFilterOnlyWithAggregateFunctionError( throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
owf.prettyName) owf.prettyName, "DISTINCT")
} else if (filter.isDefined) {
throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
owf.prettyName, "FILTER clause")
} else if (ignoreNulls) { } else if (ignoreNulls) {
owf match { owf match {
case lead: Lead => case lead: Lead =>
@ -2145,8 +2151,8 @@ class Analyzer(override val catalogManager: CatalogManager)
case lag: Lag => case lag: Lag =>
lag.copy(ignoreNulls = ignoreNulls) lag.copy(ignoreNulls = ignoreNulls)
case _ => case _ =>
throw QueryCompilationErrors.ignoreNullsWithUnsupportedFunctionError( throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
owf.prettyName) owf.prettyName, "IGNORE NULLS")
} }
} else { } else {
owf owf
@ -2161,20 +2167,23 @@ class Analyzer(override val catalogManager: CatalogManager)
case first: First => first.copy(ignoreNulls = ignoreNulls) case first: First => first.copy(ignoreNulls = ignoreNulls)
case last: Last => last.copy(ignoreNulls = ignoreNulls) case last: Last => last.copy(ignoreNulls = ignoreNulls)
case _ => case _ =>
throw QueryCompilationErrors.ignoreNullsWithUnsupportedFunctionError( throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
agg.prettyName) agg.prettyName, "IGNORE NULLS")
} }
AggregateExpression(aggFunc, Complete, isDistinct, filter) AggregateExpression(aggFunc, Complete, isDistinct, filter)
} else { } else {
AggregateExpression(agg, Complete, isDistinct, filter) AggregateExpression(agg, Complete, isDistinct, filter)
} }
// This function is not an aggregate function, just return the resolved one. // This function is not an aggregate function, just return the resolved one.
case other if (isDistinct || filter.isDefined) => case other if isDistinct =>
throw QueryCompilationErrors.distinctOrFilterOnlyWithAggregateFunctionError( throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
other.prettyName) other.prettyName, "DISTINCT")
case other if (ignoreNulls) => case other if filter.isDefined =>
throw QueryCompilationErrors.ignoreNullsWithUnsupportedFunctionError( throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
other.prettyName) other.prettyName, "FILTER clause")
case other if ignoreNulls =>
throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
other.prettyName, "IGNORE NULLS")
case e: String2TrimExpression if arguments.size == 2 => case e: String2TrimExpression if arguments.size == 2 =>
if (trimWarningEnabled.get) { if (trimWarningEnabled.get) {
log.warn("Two-parameter TRIM/LTRIM/RTRIM function signatures are deprecated." + log.warn("Two-parameter TRIM/LTRIM/RTRIM function signatures are deprecated." +

View file

@ -41,7 +41,8 @@ case class ResolveHigherOrderFunctions(catalog: SessionCatalog) extends Rule[Log
filter.foreach(_.failAnalysis("FILTER predicate specified, " + filter.foreach(_.failAnalysis("FILTER predicate specified, " +
s"but ${func.prettyName} is not an aggregate function")) s"but ${func.prettyName} is not an aggregate function"))
if (ignoreNulls) { if (ignoreNulls) {
throw QueryCompilationErrors.ignoreNullsWithUnsupportedFunctionError(func.prettyName) throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
func.prettyName, "IGNORE NULLS")
} }
func func
case other => other.failAnalysis( case other => other.failAnalysis(

View file

@ -184,22 +184,22 @@ class AnalysisErrorSuite extends AnalysisTest {
errorTest( errorTest(
"distinct function", "distinct function",
CatalystSqlParser.parsePlan("SELECT hex(DISTINCT a) FROM TaBlE"), CatalystSqlParser.parsePlan("SELECT hex(DISTINCT a) FROM TaBlE"),
"DISTINCT or FILTER specified, but hex is not an aggregate function" :: Nil) "Function hex does not support DISTINCT" :: Nil)
errorTest( errorTest(
"non aggregate function with filter predicate", "non aggregate function with filter predicate",
CatalystSqlParser.parsePlan("SELECT hex(a) FILTER (WHERE c = 1) FROM TaBlE2"), CatalystSqlParser.parsePlan("SELECT hex(a) FILTER (WHERE c = 1) FROM TaBlE2"),
"DISTINCT or FILTER specified, but hex is not an aggregate function" :: Nil) "Function hex does not support FILTER clause" :: Nil)
errorTest( errorTest(
"distinct window function", "distinct window function",
CatalystSqlParser.parsePlan("SELECT percent_rank(DISTINCT a) OVER () FROM TaBlE"), CatalystSqlParser.parsePlan("SELECT percent_rank(DISTINCT a) OVER () FROM TaBlE"),
"DISTINCT or FILTER specified, but percent_rank is not an aggregate function" :: Nil) "Function percent_rank does not support DISTINCT" :: Nil)
errorTest( errorTest(
"window function with filter predicate", "window function with filter predicate",
CatalystSqlParser.parsePlan("SELECT percent_rank(a) FILTER (WHERE c > 1) OVER () FROM TaBlE2"), CatalystSqlParser.parsePlan("SELECT percent_rank(a) FILTER (WHERE c > 1) OVER () FROM TaBlE2"),
"DISTINCT or FILTER specified, but percent_rank is not an aggregate function" :: Nil) "Function percent_rank does not support FILTER clause" :: Nil)
errorTest( errorTest(
"higher order function with filter predicate", "higher order function with filter predicate",