cloud-fan commented on code in PR #59072:
URL: https://github.com/apache/spark/pull/59072#discussion_r4115778971
##########
sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala:
##########
@@ -439,6 +439,21 @@ class CsvFunctionsSuite extends SharedSparkSession {
checkAnswer(df1.selectExpr("to_csv(a)"), Row("1") :: Nil)
}
+ test("to_csv with non-foldable options") {
Review Comment:
Added JSON and XML coverage and a lambda-variable regression test. I am
targeting `master` and `branch-4.x` only, so I am not planning a `branch-4.2`
backport.
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -4841,6 +4841,11 @@
"Invalid options:"
],
"subClass" : {
+ "NON_FOLDABLE" : {
+ "message" : [
+ "Must use a foldable `map()` function for options."
Review Comment:
Agreed. The updated patch reuses `NON_FOLDABLE_ARGUMENT` and
`QueryCompilationErrors.nonFoldableArgumentError`, passes the function name
from all nine CSV/JSON/XML call sites, and removes
`INVALID_OPTIONS.NON_FOLDABLE`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ExprUtils.scala:
##########
@@ -65,6 +65,9 @@ object ExprUtils extends EvalHelper with QueryErrorsBase {
StringTypeWithCollation(supportsTrimCollation = true),
StringTypeWithCollation(supportsTrimCollation = true))
.acceptsType(m.dataType) =>
+ if (!m.foldable) {
Review Comment:
Thanks for identifying these compatibility cases. I agree that requiring
`foldable` is the consistent contract used by other functions. The updated
patch enforces it by default, adds
`spark.sql.legacy.allowNonFoldableOptions=true` to preserve the old
analysis-time evaluation when needed, and documents the migration in the 4.4
guide. The tests cover `nvl`, `elt`, higher-order `transform`, and the guarded
`raise_error` case under both the default and legacy behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
