Skip to content

[SPARK-51805] [SQL] Get function with improper argument should throw proper exception instead of an internal one #50590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mihailoale-db wants to merge 1 commit into from
Closed

[SPARK-51805] [SQL] Get function with improper argument should throw proper exception instead of an internal one #50590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions ...lyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ package org.apache.spark.sql.catalyst.expressions
import org.apache.spark.QueryContext
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.analysis._
import org.apache.spark.sql.catalyst.analysis.TypeCheckResult.DataTypeMismatch
import org.apache.spark.sql.catalyst.expressions.codegen.{CodegenContext, CodeGenerator, ExprCode}
import org.apache.spark.sql.catalyst.trees.TreePattern.{EXTRACT_VALUE, TreePattern}
import org.apache.spark.sql.catalyst.util.{quoteIdentifier, ArrayData, GenericArrayData, MapData, TypeUtils}
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryExecutionErrors}
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryErrorsBase, QueryExecutionErrors}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._

Expand Down Expand Up @@ -90,7 +91,7 @@ object ExtractValue {
}
}

trait ExtractValue extends Expression {
trait ExtractValue extends Expression with QueryErrorsBase {
override def nullIntolerant: Boolean = true
final override val nodePatterns: Seq[TreePattern] = Seq(EXTRACT_VALUE)
val child: Expression
Expand Down Expand Up @@ -314,6 +315,30 @@ case class GetArrayItem(
})
}

override def checkInputDataTypes(): TypeCheckResult = {
(left.dataType, right.dataType) match {
case (_: ArrayType, e2) if !e2.isInstanceOf[IntegralType] =>
Copy link
Contributor

@cloud-fan cloud-fan Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a test to examine this branch?

Copy link
Contributor Author

@mihailoale-db mihailoale-db Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they have existed before (please check array.sql). But on the other hand select get(array(1),null) passes because GetArrayItem extends ExpectsInputTypes and because of that NullType is casted to IntegerType. Should we remove this check as it is noop?

Copy link
Contributor Author

@mihailoale-db mihailoale-db Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it for safety (so we don't throw internal error if we somehow fail to apply type coercion rule). @cloud-fan

DataTypeMismatch(
errorSubClass = "UNEXPECTED_INPUT_TYPE",
messageParameters = Map(
"paramIndex" -> ordinalNumber(1),
"requiredType" -> toSQLType(IntegralType),
"inputSql" -> toSQLExpr(right),
"inputType" -> toSQLType(right.dataType))
)
case (e1, _) if !e1.isInstanceOf[ArrayType] =>
DataTypeMismatch(
errorSubClass = "UNEXPECTED_INPUT_TYPE",
messageParameters = Map(
"paramIndex" -> ordinalNumber(0),
"requiredType" -> toSQLType(TypeCollection(ArrayType)),
"inputSql" -> toSQLExpr(left),
"inputType" -> toSQLType(left.dataType))
)
case _ => TypeCheckResult.TypeCheckSuccess
}
}

override protected def withNewChildrenInternal(
newLeft: Expression, newRight: Expression): GetArrayItem =
copy(child = newLeft, ordinal = newRight)
Expand Down
192 changes: 192 additions & 0 deletions sql/core/src/test/resources/sql-tests/analyzer-results/array.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,198 @@ Project [get(array(1, 2, 3), -1) AS get(array(1, 2, 3), -1)#x]
+- OneRowRelation


-- !query
select get(1, 0)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"1\"",
"inputType" : "\"INT\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"1[0]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 16,
"fragment" : "get(1, 0)"
} ]
}


-- !query
select get(1, -1)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"1\"",
"inputType" : "\"INT\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"1[-1]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 17,
"fragment" : "get(1, -1)"
} ]
}


-- !query
select get('1', 0)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"1\"",
"inputType" : "\"STRING\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"1[0]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 18,
"fragment" : "get('1', 0)"
} ]
}


-- !query
select get('1', -1)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"1\"",
"inputType" : "\"STRING\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"1[-1]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 19,
"fragment" : "get('1', -1)"
} ]
}


-- !query
select get(null, 0)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"NULL\"",
"inputType" : "\"VOID\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"NULL[0]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 19,
"fragment" : "get(null, 0)"
} ]
}


-- !query
select get(null, -1)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"NULL\"",
"inputType" : "\"VOID\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"NULL[-1]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 20,
"fragment" : "get(null, -1)"
} ]
}


-- !query
select get(null, null)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"NULL\"",
"inputType" : "\"VOID\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"NULL[NULL]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 22,
"fragment" : "get(null, null)"
} ]
}


-- !query
select get(CAST (null AS string), 0)
-- !query analysis
org.apache.spark.sql.catalyst.ExtendedAnalysisException
{
"errorClass" : "DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE",
"sqlState" : "42K09",
"messageParameters" : {
"inputSql" : "\"CAST(NULL AS STRING)\"",
"inputType" : "\"STRING\"",
"paramIndex" : "first",
"requiredType" : "(\"ARRAY\")",
"sqlExpr" : "\"CAST(NULL AS STRING)[0]\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 8,
"stopIndex" : 36,
"fragment" : "get(CAST (null AS string), 0)"
} ]
}


-- !query
select array_insert(array(1, 2, 3), 3, 4)
-- !query analysis
Expand Down
Loading