HyukjinKwon commented on code in PR #59422:
URL: https://github.com/apache/spark/pull/59422#discussion_r4274686774
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4411,6 +4411,17 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val LEGACY_REGEXP_INSTR_IGNORE_GROUP_INDEX =
+ buildConf("spark.sql.legacy.regexpInstrIgnoreGroupIndex")
+ .internal()
+ .doc("When true, `regexp_instr(str, regexp, idx)` ignores `idx` and
returns the " +
+ "position of the entire match, as in Spark 4.3 and earlier. When
false, it returns the " +
+ "position of the regex group `idx`, and fails if `idx` is negative or
greater than " +
+ "the number of groups.")
+ .version("4.4.0")
Review Comment:
**Non-blocking (P2):** This config is missing `.withBindingPolicy(...)`.
`SparkConfigBindingPolicySuite` requires one on every new config, and that
suite is what fails in the `Build modules: hive - other tests` job.
The policy also changes behavior here. `RegExpInStr` reads the flag when the
function is resolved. A persisted view or SQL function body is resolved under
`View.effectiveSQLConf`, which copies a session value only when
`SQLConf.isSessionBindingPolicy` is true, i.e. for SESSION or NOT_APPLICABLE.
With no policy, take a view such as `CREATE VIEW v AS SELECT regexp_instr(s,
'abc', 1) AS p FROM t` created before the upgrade. It keeps failing with
`REGEX_GROUP_INDEX` even after `SET
spark.sql.legacy.regexpInstrIgnoreGroupIndex=true`, so the remedy in the
migration guide does not work for stored queries.
The `ConfigBindingPolicy` scaladoc recommends SESSION for bug-fix flags, and
recent result-changing legacy flags such as
`spark.sql.legacy.literal.pickMinimumPrecision` use it. PERSISTED would also
make the suite pass, but views created before 4.4 never captured this key, so
they would still get the new behavior.
```suggestion
.version("4.4.0")
.withBindingPolicy(ConfigBindingPolicy.SESSION)
```
**Verification:**
- **Behavior:** A persisted view whose body calls regexp_instr with a
nonzero idx returns whole-match positions when the session sets the legacy flag
to true, and group positions (or the group-index error) when it does not.
- **Compatibility:** Every registered SQL config declares a binding policy,
and the frozen list of configs without one is not extended.
- **Regression:** Existing default-mode and legacy-mode regexp_instr results
at the expression, DataFrame, and SQL golden levels are unchanged.
--
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]
