Skip to content
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

[SPARK-38225][SQL] Adjust input format of function to_binary #35533

Closed
wants to merge 7 commits into from
Closed

[SPARK-38225][SQL] Adjust input format of function to_binary #35533

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5c01f94
for wrong format input
xinrong-meng Feb 15, 2022
2869cb1
test
xinrong-meng Feb 15, 2022
98634d7
trigger test
xinrong-meng Feb 16, 2022
8be5db9
rmv base2 format support
xinrong-meng Feb 16, 2022
4b93726
fail when format is cast(null as int)
xinrong-meng Feb 17, 2022
6d94e00
rmv unnecessary chk
xinrong-meng Feb 17, 2022
75494c3
fix
xinrong-meng Feb 18, 2022
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
9 changes: 5 additions & 4 deletions ...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2565,15 +2565,15 @@ case class ToBinary(expr: Expression, format: Option[Expression], child: Express
case lit if lit.foldable =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd put the string type check here, as to_binary(str_col, cast(null as int)) should fail instead of returning null.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense.

May I confirm that the case below is acceptable?

spark-sql> select to_binary(cast(null as int), 'utf-8');
NULL

Copy link
Contributor

Choose a reason for hiding this comment

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

yea, the type coercion can cast any atomic type to string type implicitly.

val value = lit.eval()
if (value == null) Literal(null, BinaryType)
else {
else if (value.isInstanceOf[UTF8String]) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Is it possible for value to be an instance of java.lang.String?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, it can only be UTF8String

Copy link
Contributor

Choose a reason for hiding this comment

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

seems we can remove this check now, as we have checked the data type ahead

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense! Removed.

value.asInstanceOf[UTF8String].toString.toLowerCase(Locale.ROOT) match {
case "hex" => Unhex(expr)
case "utf-8" => Encode(expr, Literal("UTF-8"))
case "base64" => UnBase64(expr)
case "base2" => Cast(expr, BinaryType)
case _ => lit
}
}
} else lit

case other => other
}
Expand All @@ -2591,8 +2591,9 @@ case class ToBinary(expr: Expression, format: Option[Expression], child: Express
def checkFormat(lit: Expression) = {
if (lit.foldable) {
val value = lit.eval()
value == null || Seq("hex", "utf-8", "base64", "base2").contains(
value.asInstanceOf[UTF8String].toString.toLowerCase(Locale.ROOT))
value == null || (value.isInstanceOf[UTF8String] &&
Seq("hex", "utf-8", "base64", "base2").contains(
value.asInstanceOf[UTF8String].toString.toLowerCase(Locale.ROOT)))
} else false
}

Expand Down
1 change: 1 addition & 0 deletions sql/core/src/test/resources/sql-tests/inputs/string-functions.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,4 @@ select to_binary(null, 'utf-8');
select to_binary(null, null);
select to_binary(null, cast(null as string));
select to_binary('abc', 'invalidFormat');
select to_binary('abc', 1);
11 changes: 10 additions & 1 deletion sql/core/src/test/resources/sql-tests/results/ansi/string-functions.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
-- Automatically generated by SQLQueryTestSuite
-- Number of queries: 116
-- Number of queries: 117


-- !query
Expand Down Expand Up @@ -937,3 +937,12 @@ struct<>
-- !query output
org.apache.spark.sql.AnalysisException
cannot resolve 'to_binary('abc', 'invalidFormat')' due to data type mismatch: Unsupported encoding format: Some(invalidFormat). The format has to be a case-insensitive string literal of 'hex', 'utf-8', 'base2', or 'base64'; line 1 pos 7


-- !query
select to_binary('abc', 1)
-- !query schema
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Copy link
Member Author

Choose a reason for hiding this comment

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

Intentionally failed query.

cannot resolve 'to_binary('abc', 1)' due to data type mismatch: Unsupported encoding format: Some(1). The format has to be a case-insensitive string literal of 'hex', 'utf-8', 'base2', or 'base64'; line 1 pos 7
11 changes: 10 additions & 1 deletion sql/core/src/test/resources/sql-tests/results/string-functions.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
-- Automatically generated by SQLQueryTestSuite
-- Number of queries: 116
-- Number of queries: 117


-- !query
Expand Down Expand Up @@ -933,3 +933,12 @@ struct<>
-- !query output
org.apache.spark.sql.AnalysisException
cannot resolve 'to_binary('abc', 'invalidFormat')' due to data type mismatch: Unsupported encoding format: Some(invalidFormat). The format has to be a case-insensitive string literal of 'hex', 'utf-8', 'base2', or 'base64'; line 1 pos 7


-- !query
select to_binary('abc', 1)
-- !query schema
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Copy link
Member Author

Choose a reason for hiding this comment

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

Intentionally failed query.

cannot resolve 'to_binary('abc', 1)' due to data type mismatch: Unsupported encoding format: Some(1). The format has to be a case-insensitive string literal of 'hex', 'utf-8', 'base2', or 'base64'; line 1 pos 7