[SPARK-48041][SQL] Avoid call conf.resolver repeated in TableOutputResolver #46279
Conversation
|
@dongjoon-hyun PTAL |
| @@ -3816,7 +3816,7 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor | |||
| case u: UnresolvedFieldPosition => u.position match { | |||
| case after: After => | |||
| val allFields = parentSchema.fieldNames ++ fieldsAdded | |||
| allFields.find(n => conf.resolver(n, after.column())) match { | |||
| allFields.find(n => resolver(n, after.column())) match { | |||
There was a problem hiding this comment.
There's one more below L3895
|
Can you fill the PR description please? |
xuzifu666
changed the title
Done @HyukjinKwon |
xuzifu666
changed the title
|
Is there any other outstanding issue similar to SPARK-48010 and this one? |
yes，similar Sences@yaooqinn |
| expected.forall(e => queryOutput.exists(p => conf.resolver(p.name, e.name))) && | ||
| expected.zip(queryOutput).exists(e => !conf.resolver(e._1.name, e._2.name))) { | ||
| expected.forall(e => queryOutput.exists(p => resolver(p.name, e.name))) && | ||
| expected.zip(queryOutput).exists(e => !resolver(e._1.name, e._2.name))) { |
There was a problem hiding this comment.
Is this okay? What happens spark.sql.caseSensitive is changed during session?
| @@ -219,7 +221,7 @@ object TableOutputResolver extends SQLConfHelper with Logging { | |||
| fillDefaultValue: Boolean = false): Seq[NamedExpression] = { | |||
| val matchedCols = mutable.HashSet.empty[String] | |||
| val reordered = expectedCols.flatMap { expectedCol => | |||
| val matched = inputCols.filter(col => conf.resolver(col.name, expectedCol.name)) | |||
| val matched = inputCols.filter(col => resolver(col.name, expectedCol.name)) | |||
There was a problem hiding this comment.
This conf is given by the method parameter. I'm not sure this is correct optimization.
There was a problem hiding this comment.
To @xuzifu666 , TableOutputResolver Scala object is created once for JVM, isn't it?
I'm wondering if this proposal is correct or not. Basically, the user can change conf.resolver at any time, but this proposal reads it only once.
WDYT?
What changes were proposed in this pull request?
This PR aim to call conf.resolver for each call in Analyzer.
Why are the changes needed?
conf.resolver be called repeated in TableOutputResolver，with change can reuse the same resolver.
Does this PR introduce any user-facing change?
none
How was this patch tested?
no need
Was this patch authored or co-authored using generative AI tooling?
no