Skip to content

[SPARK-44983][SQL] Convert binary to string by to_char for the formats: hex, base64, utf-8 #42632

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
MaxGekk wants to merge 6 commits into from
Closed

[SPARK-44983][SQL] Convert binary to string by to_char for the formats: hex, base64, utf-8 #42632

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
76af197
Fix examples
MaxGekk Aug 23, 2023
89e7660
Add a test for _LEGACY_ERROR_TEMP_1100
MaxGekk Aug 26, 2023
b7987e1
Add tests for errors
MaxGekk Aug 26, 2023
c13f182
Trigger build
MaxGekk Aug 26, 2023
1fa15d7
Add more tests
MaxGekk Aug 28, 2023
ec790bb
Trigger build
MaxGekk Aug 28, 2023
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
Prev Previous commit
Next Next commit
Add tests for errors
  • Loading branch information
@MaxGekk
MaxGekk committed Aug 26, 2023
commit b7987e10a6c2d6685f6c2c28e02c3e0e6d7e2d1a
5 changes: 5 additions & 0 deletions common/utils/src/main/resources/error/error-classes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,11 @@
"expects a binary value with 16, 24 or 32 bytes, but got <actualLength> bytes."
]
},
"BINARY_FORMAT" : {
"message" : [
"expects one of binary formats 'base64', 'hex', 'utf-8', but got <invalidFormat>."
]
},
"DATETIME_UNIT" : {
"message" : [
"expects one of the units without quotes YEAR, QUARTER, MONTH, WEEK, DAY, DAYOFYEAR, HOUR, MINUTE, SECOND, MILLISECOND, MICROSECOND, but got the string literal <invalidValue>."
Expand Down
4 changes: 4 additions & 0 deletions docs/sql-error-conditions-invalid-parameter-value-error-class.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ supports 16-byte CBC IVs and 12-byte GCM IVs, but got `<actualLength>` bytes for

expects a binary value with 16, 24 or 32 bytes, but got `<actualLength>` bytes.

## BINARY_FORMAT

expects one of binary formats 'base64', 'hex', 'utf-8', but got `<invalidFormat>`.

## DATETIME_UNIT

expects one of the units without quotes YEAR, QUARTER, MONTH, WEEK, DAY, DAYOFYEAR, HOUR, MINUTE, SECOND, MILLISECOND, MICROSECOND, but got the string literal `<invalidValue>`.
Expand Down
5 changes: 3 additions & 2 deletions ...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/numberFormatExpressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ object ToCharacterBuilder extends ExpressionBuilder {
if (expressions.length == 2) {
val (inputExpr, format) = (expressions(0), expressions(1))
Copy link
Contributor

@cloud-fan cloud-fan Aug 28, 2023

Choose a reason for hiding this comment

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

this is a bit weird, we can just write 2 lines

val inputExpr = expressions(0)
val format = expressions(1)

Copy link
Member Author

@MaxGekk MaxGekk Aug 28, 2023

Choose a reason for hiding this comment

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

Don't think it is the right approach to split semantically one operation: "assign names".

inputExpr.dataType match {
case _: DatetimeType => DateFormatClass(inputExpr, expressions(1))
case _: DatetimeType => DateFormatClass(inputExpr, format)
case _: BinaryType =>
if (!(format.dataType == StringType && format.foldable)) {
throw QueryCompilationErrors.requireLiteralParameter(funcName, "format", "string")
Expand All @@ -253,8 +253,9 @@ object ToCharacterBuilder extends ExpressionBuilder {
case "base64" => Base64(inputExpr)
case "hex" => Hex(inputExpr)
case "utf-8" => new Decode(Seq(inputExpr, format))
case invalid => throw QueryCompilationErrors.binaryFormatError(funcName, invalid)
}
case _ => ToCharacter(inputExpr, expressions(1))
case _ => ToCharacter(inputExpr, format)
}
} else {
throw QueryCompilationErrors.wrongNumArgsError(funcName, Seq(2), numArgs)
Expand Down
9 changes: 9 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,15 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase with Compilat
"functionName" -> toSQLId("format_string")))
}

def binaryFormatError(funcName: String, invalidFormat: String): Throwable = {
new AnalysisException(
Copy link
Contributor

@cloud-fan cloud-fan Aug 28, 2023

Choose a reason for hiding this comment

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

My feeling is that, if the error is only thrown in one place, we don't need to add a method here, just throw new AnalysisException... in the caller side.

Copy link
Member Author

@MaxGekk MaxGekk Aug 28, 2023

Choose a reason for hiding this comment

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

One of purposes of gathering exceptions to Query*Errors to don't depend on forming exceptions: errors classes, quoting, context and so on. The caller has to provide only valuable info and don't worry about any technical stuff.

Don't think that this PR is right place to begin don't do that.

errorClass = "INVALID_PARAMETER_VALUE.BINARY_FORMAT",
messageParameters = Map(
"parameter" -> toSQLId("format"),
"functionName" -> toSQLId(funcName),
"invalidFormat" -> toSQLValue(invalidFormat, StringType)))
}

def unorderablePivotColError(pivotCol: Expression): Throwable = {
new AnalysisException(
errorClass = "INCOMPARABLE_PIVOT_COLUMN",
Expand Down
20 changes: 20 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,26 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
"argName" -> "format",
"funcName" -> "to_char",
"requiredType" -> "string"))
checkError(
exception = intercept[AnalysisException] {
df2.select(to_char(col("input"), lit("invalid_format"))).collect()
},
errorClass = "INVALID_PARAMETER_VALUE.BINARY_FORMAT",
parameters = Map(
"parameter" -> "`format`",
"functionName" -> "`to_char`",
"invalidFormat" -> "'invalid_format'"))
checkError(
exception = intercept[AnalysisException] {
sql("select to_char('a', 'b', 'c')")
},
errorClass = "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
parameters = Map(
"functionName" -> "`to_char`",
"expectedNum" -> "2",
"actualNum" -> "3",
"docroot" -> SPARK_DOC_ROOT),
context = ExpectedContext("", "", 7, 28, "to_char('a', 'b', 'c')"))
}

test("to_varchar") {
Expand Down