HyukjinKwon commented on code in PR #59072:
URL: https://github.com/apache/spark/pull/59072#discussion_r4118155254
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ExprUtils.scala:
##########
@@ -59,12 +60,17 @@ object ExprUtils extends EvalHelper with QueryErrorsBase {
dataType.asInstanceOf[StructType]
}
- def convertToMapData(exp: Expression): Map[String, String] = exp match {
+ def convertToMapData(exp: Expression, functionName: String): Map[String,
String] = exp match {
case m: CreateMap
if AbstractMapType(
StringTypeWithCollation(supportsTrimCollation = true),
StringTypeWithCollation(supportsTrimCollation = true))
.acceptsType(m.dataType) =>
+ if (!m.foldable &&
Review Comment:
`!m.foldable` is broader than the failure being fixed. `RuntimeReplaceable`
(e.g. `nvl`, `ifnull`) doesn't override `foldable`, so `map('delimiter',
nvl(NULL, '|'))` is now rejected even though `m.eval()` evaluated it fine in
4.3. Would a check aimed at what `eval()` can't handle at analysis time
(attribute, lambda-variable or outer references, or nondeterminism) fix the
`INTERNAL_ERROR` without needing the legacy flag for constant options?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -7557,6 +7557,15 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val LEGACY_ALLOW_NON_FOLDABLE_OPTIONS =
Review Comment:
This entry needs `.withBindingPolicy(...)`, or
`SparkConfigBindingPolicySuite` fails. It's currently failing on this head with
"The following configs do not have bindingPolicy field set". The suite's
exceptions file is frozen, so the policy has to be declared here. Since this
changes analysis behavior per session, `ConfigBindingPolicy.SESSION` seems like
the natural choice.
##########
AGENTS.md:
##########
@@ -101,7 +101,7 @@ These are combined with a base above rather than used on
their own:
## Build and Test
-Build and tests can take a long time. If the user explicitly asked to run
tests, run them. Otherwise (you are running tests on your own to verify a
change), first ask the user if they have more changes to make.
+Build and tests can take a long time, so prefer the narrowest relevant build
or test command.
Review Comment:
This comes from the separate `[MINOR][INFRA]` commit and isn't related to
SPARK-59801. Could it be split into its own PR?
--
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]
