szehon-ho commented on code in PR #57610:
URL: https://github.com/apache/spark/pull/57610#discussion_r3669944995
##########
sql/pipelines/src/main/scala/org/apache/spark/sql/pipelines/autocdc/ChangeArgs.scala:
##########
@@ -129,6 +133,13 @@ private[pipelines] object CaseSensitivityLabels {
def of(caseSensitive: Boolean): String =
if (caseSensitive) CaseSensitive else CaseInsensitive
+
+ /**
+ * Maps a [[Resolver]] to its user-facing label. A resolver is
case-insensitive iff it treats
+ * two identifiers differing only in case as equal, so we classify it by
probing with such a
+ * pair rather than by reference identity; the label then stays correct for
any resolver.
+ */
+ def of(resolver: Resolver): String = of(caseSensitive = !resolver("a", "A"))
Review Comment:
Nit: probing with `"a"` / `"A"` feels a bit awkward for what is only an
error-message label.
Spark already classifies resolvers by reference identity against the two
singletons — see `SchemaUtils.isCaseSensitiveAnalysis` — and `SQLConf.resolver`
only ever returns those. Something like:
```scala
def of(resolver: Resolver): String =
of(caseSensitive = resolver == caseSensitiveResolution)
```
(or the same check at the one call site, without a new overload) would match
that pattern. The "works for any resolver" rationale seems oversold here:
AutoCDC callers pass `conf.resolver` / the test singletons, and SchemaUtils
treats anything else as unreachable. Also `Flow.scala` still uses
`of(caseSensitiveAnalysis)`, so this invents a third classification strategy
alongside the boolean overload.
--
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]
