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: [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]

Reply via email to