MaxGekk commented on a change in pull request #31265:
URL: https://github.com/apache/spark/pull/31265#discussion_r563509932



##########
File path: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala
##########
@@ -1680,4 +1680,20 @@ abstract class SessionCatalogSuite extends AnalysisTest 
with Eventually {
       }
     }
   }
+
+  test("SPARK-34197: refreshTable should not invalidate the relation cache for 
temporary views") {
+    withBasicCatalog { catalog =>
+      catalog.createTempView("tbl1", Range(1, 10, 1, 10), false)
+      val qualifiedName1 = QualifiedTableName("default", "tbl1")
+      catalog.cacheTable(qualifiedName1, Range(1, 10, 1, 10))
+      catalog.refreshTable(TableIdentifier("tbl1"))
+      assert(catalog.getCachedTable(qualifiedName1) != null)
+
+      catalog.createGlobalTempView("tbl2", Range(2, 10, 1, 10), false)
+      val qualifiedName2 = 
QualifiedTableName(catalog.globalTempViewManager.database, "tbl2")
+      catalog.cacheTable(qualifiedName2, Range(2, 10, 1, 10))

Review comment:
       The test is not about to put a temp view into the relation cache. It is 
about existence of a temp view and a table in the relation cache with the same 
name. And it checks that refreshing the view, does not invalidate table's entry 
in the cache.
   
   This is a unit test, and it should check all possible input parameters/cases 
for the functions. If you are saying that some situations are not possible 
outside of functions, that does not mean we should't test them. 




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to