[GitHub] [spark] bogdanghit commented on a change in pull request #34453: [SPARK-37173][SQL] SparkGetFunctionOperation return builtin function only once

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

bogdanghit commented on a change in pull request #34453:
URL: https://github.com/apache/spark/pull/34453#discussion_r770077112



##########
File path: docs/sql-migration-guide.md
##########
@@ -54,6 +54,8 @@ license: |
 
   - Since Spark 3.3, nulls are written as empty strings in CSV data source by default. In Spark 3.2 or earlier, nulls were written as empty strings as quoted empty strings, `""`. To restore the previous behavior, set `nullValue` to `""`.
 
+  - Since Spark 3.3, Spark Thrift Server will return databases' system functions metadata only once, and Spark will change function schema as `SYSTEM`. In Spark 3.2 or earlier, Spark Thrift Server will return system functions metadata for all databases. To restore the behavior before Spark 3.3, yo you can set `spark.sql.thriftserver.separateDisplaySystemFunctions` to `false`.

Review comment:
       nits: 
   1. `will return the available system function metadata for databases only once`
   2. maybe "all system system functions metadata for each database" to emphasise it results in duplicates?
   3. "... for all databases which results in duplicates". 
   4. typo "yo you"

##########
File path: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/SparkMetadataOperationSuite.scala
##########
@@ -236,6 +240,23 @@ class SparkMetadataOperationSuite extends HiveThriftServer2TestBase {
       checkResult(metaData.getFunctions(null, "default", "shift*"),
         Seq("shiftleft", "shiftright", "shiftrightunsigned"))
       checkResult(metaData.getFunctions(null, "default", "upPer"), Seq("upper"))
+
+      statement.execute(s"SET ${SQLConf.THRIFTSERVER_SEPARATE_DISPLAY_SYSTEM_FUNCTION.key}=true")

Review comment:
       Can we add a test with two schemas and run an unfiltered getFunctions call to show that previously we'd see duplicates, whereas now the functions are unique?

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -1158,6 +1158,14 @@ object SQLConf {
     .intConf
     .createWithDefault(200)
 
+  val THRIFTSERVER_SEPARATE_DISPLAY_SYSTEM_FUNCTION =
+    buildConf("spark.sql.thriftserver.separateDisplaySystemFunctions")
+      .doc("When true, Spark Thrift Server will return databases' system functions metadata " +

Review comment:
       nits:
   1. `will return the available system function metadata for databases only once`
   2. `will set the function schema as`
   
   What does separate display mean here? Would `spark.sql.thriftserver.uniqueSystemFunctions`?




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