[GitHub] [spark] maropu commented on a change in pull request #30097: [SPARK-33140][SQL] remove SQLConf and SparkSession in all sub-class of Rule[QueryPlan]

[GitBox <gi...@apache.org>]

maropu commented on a change in pull request #30097:
URL: https://github.com/apache/spark/pull/30097#discussion_r509817059



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -988,11 +988,11 @@ class Analyzer(
       case view @ View(desc, _, child) if !child.resolved =>
         // Resolve all the UnresolvedRelations and Views in the child.
         val newChild = AnalysisContext.withAnalysisContext(desc.viewCatalogAndNamespace) {
-          if (AnalysisContext.get.nestedViewDepth > conf.maxNestedViewDepth) {
+          if (AnalysisContext.get.nestedViewDepth > SQLConf.get.maxNestedViewDepth) {
             view.failAnalysis(s"The depth of view ${desc.identifier} exceeds the maximum " +
-              s"view resolution depth (${conf.maxNestedViewDepth}). Analysis is aborted to " +
-              s"avoid errors. Increase the value of ${SQLConf.MAX_NESTED_VIEW_DEPTH.key} to work " +
-              "around this.")
+              s"view resolution depth (${SQLConf.get.maxNestedViewDepth}). " +
+              s"Analysis is aborted to avoid errors. " +

Review comment:
       nit: remove `s`

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -182,7 +182,7 @@ class Analyzer(
 

Review comment:
       Could we remove `conf: SQLConf` in the constructor of `Analyzer`?

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveInlineTables.scala
##########
@@ -28,7 +28,8 @@ import org.apache.spark.sql.types.{StructField, StructType}
 /**
  * An analyzer rule that replaces [[UnresolvedInlineTable]] with [[LocalRelation]].
  */
-case class ResolveInlineTables(conf: SQLConf) extends Rule[LogicalPlan] with CastSupport {
+object ResolveInlineTables extends Rule[LogicalPlan] with CastSupport {
+  override def conf: SQLConf = SQLConf.get

Review comment:
       Instead, could we define `final def conf: SQLConf = SQLConf.get` in the base class `Rule` if this is a common pattern?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org