amaliujia commented on code in PR #40496:
URL: https://github.com/apache/spark/pull/40496#discussion_r1144038811


##########
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestHelper.scala:
##########
@@ -59,10 +61,39 @@ trait SQLQueryTestHelper extends Logging {
    */
   protected def getNormalizedQueryAnalysisResult(
       session: SparkSession, sql: String): (String, Seq[String]) = {
+    // Note that creating the following DataFrame includes eager execution for 
commands that create
+    // objects such as views. Therefore any following queries that reference 
these objects should
+    // find them in the catalog.
     val df = session.sql(sql)
     val schema = df.schema.catalogString
-    // Get the answer, but also get rid of the #1234 expression IDs that show 
up in analyzer plans.
-    (schema, Seq(replaceNotIncludedMsg(df.queryExecution.analyzed.toString)))
+    val analyzed = df.queryExecution.analyzed
+    // Determine if the analyzed plan contains any nondeterministic 
expressions.
+    var deterministic = true
+    analyzed.transformAllExpressionsWithSubqueries {
+      case expr: CurrentDate =>
+        deterministic = false
+        expr
+      case expr: CurrentTimestampLike =>
+        deterministic = false
+        expr
+      case expr: CurrentUser =>
+        deterministic = false
+        expr
+      case expr: Literal if expr.dataType == DateType || expr.dataType == 
TimestampType =>
+        deterministic = false
+        expr
+      case expr if !expr.deterministic =>
+        deterministic = false
+        expr
+    }
+    if (deterministic) {
+      // Perform query analysis, but also get rid of the #1234 expression IDs 
that show up in the
+      // resolved plans.
+      (schema, Seq(replaceNotIncludedMsg(analyzed.toString)))
+    } else {
+      // The analyzed plan is nondeterministic so elide it from the result to 
keep tests reliable.
+      (schema, Seq("[Analyzer test output redacted due to nondeterminism]"))
+    }

Review Comment:
   I was wondering if this code is used in non testing environment and I tried 
a code search and it seems only being used with test suites.
   
   Basically I was not sure in production if this part is also used then the 
`if else` actually changes the output.
   
   At least code search shows this is not a concern.



-- 
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-unsubscr...@spark.apache.org

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


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

Reply via email to