cloud-fan commented on code in PR #53830:
URL: https://github.com/apache/spark/pull/53830#discussion_r2699523307
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala:
##########
@@ -789,4 +792,23 @@ class JDBCTableCatalogSuite extends QueryTest with
SharedSparkSession {
assert(plan.isInstanceOf[InMemoryTableScanExec])
}
}
+
+ test("loadTable execute single query") {
Review Comment:
this test have very little value, as the code is very obvious that `def
loadTable(ident: Identifier)` calls `def loadTable(ident: Identifier, conn:
Connection)` once. please do not add test only for test.
What we really want to check is the number of JDBC API calls. Do we have
metrics for the number of JDBC connection creation? The way we test hive
catalog is to check hive catalog metrics, see examples:
https://github.com/apache/spark/blob/master/sql/hive/src/test/scala/org/apache/spark/sql/hive/PartitionedTablePerfStatsSuite.scala#L138-L166
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala:
##########
@@ -789,4 +792,23 @@ class JDBCTableCatalogSuite extends QueryTest with
SharedSparkSession {
assert(plan.isInstanceOf[InMemoryTableScanExec])
}
}
+
+ test("loadTable execute single query") {
Review Comment:
this test has very little value, as the code is very obvious that `def
loadTable(ident: Identifier)` calls `def loadTable(ident: Identifier, conn:
Connection)` once. please do not add test only for test.
What we really want to check is the number of JDBC API calls. Do we have
metrics for the number of JDBC connection creation? The way we test hive
catalog is to check hive catalog metrics, see examples:
https://github.com/apache/spark/blob/master/sql/hive/src/test/scala/org/apache/spark/sql/hive/PartitionedTablePerfStatsSuite.scala#L138-L166
--
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]
