[GitHub] [spark] sunchao commented on a change in pull request #34431: [SPARK-35437][SQL] Use expressions to filter Hive partitions at client side

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

sunchao commented on a change in pull request #34431:
URL: https://github.com/apache/spark/pull/34431#discussion_r744291089



##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HivePartitionFilteringSuite.scala
##########
@@ -62,6 +63,9 @@ class HivePartitionFilteringSuite(version: String)
     properties = Map.empty
   )
 
+  // Avoid repeatedly constructing multiple hive instances that do not use direct sql
+  private var disableDirectSqlClient: HiveClient = _

Review comment:
       hmm can you explain why this is useful? 
   
   looks like by caching the client, we just need to create 1 instead of 3 clients now, for each separate run on a Hive version (2 initialization in the existing code + 1 initialization in the new test case `getPartitionsByFilter: ds=20170101 when $fallbackKey=true`, is that correct?
   
   I'm also curious why the OOM didn't happen before, as we were already using the `init(false)` in the tests.
   
   

##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HivePartitionFilteringSuite.scala
##########
@@ -577,6 +582,60 @@ class HivePartitionFilteringSuite(version: String)
       dateStrValue)
   }
 
+  test("getPartitionsByFilter: substr(chunk,0,1)=a") {

Review comment:
       nit: could we prepend JIRA number to these new tests?




-- 
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: reviews-unsubscribe@spark.apache.org

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