spark-test commented on code in PR #60113:
URL: https://github.com/apache/spark/pull/60113#discussion_r4475584896
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala:
##########
@@ -179,6 +179,27 @@ private[hive] class HiveClientImpl(
log"${MDC(HIVE_CLIENT_VERSION, version.fullVersion)}) is " +
log"${MDC(PATH,
conf.getVar(HiveConf.getConfVars("hive.metastore.warehouse.dir")))}")
+ // Hive 4 creates its metastore connection pool without a JDBC driver class
name, so the pool
+ // gets the driver from java.sql.DriverManager. DriverManager loads the
drivers listed in
+ // META-INF/services only once per JVM, through the context class loader of
its first caller,
+ // and returns only drivers that the caller's class loader resolves to the
same class. When an
+ // isolated client opens the metastore database itself, load the configured
JDBC driver through
+ // the client class loader so that the driver registers itself with
DriverManager.
+ if (clientLoader.isolationOn) {
Review Comment:
**Nit (P3):** Nothing in the test suite would fail if this block were
removed or its gate broken. The suites that build isolated Hive 4 clients
(`HiveClientSuites`, `HiveClientUserNameSuites`,
`HivePartitionFilteringSuites`) share one test JVM, so `DriverManager`'s state
depends on which suites ran before them. The dedicated-JVM entry in
`project/SparkBuild.scala` names the `HiveClientVersions` trait, which matches
no suite, and Hive 4.2 does not run at all on the default Java 17 CI. As the PR
description notes, `HiveClientSuite(4.2)` passes in CI only because other
suites register Spark's Derby driver first and Hive's retry then succeeds. The
temporary reproducer described under "How was this patch tested?" (an in-memory
Derby connection opened through the application class loader before any Hive
client exists) sets up exactly the ordering a regression test needs, but it was
not kept. Without such a test, a later refactor of this constructor or of the
isolation and `hive.metastor
e.uris` gate, or a Hive or Derby upgrade that moves the configured driver
class (as the Derby 10.16 upgrade did for
`org.apache.derby.jdbc.EmbeddedDriver`), could bring back `No suitable driver`
unnoticed, on master and on each backport branch.
**Recommended change:** Add a regression test to the Hive spark-submit tests
whose child application first opens an in-memory Derby connection through the
application class loader, then starts a Hive-enabled session with
spark.sql.hive.metastore.version=4.1.0 and spark.sql.hive.metastore.jars=maven
against a local Derby metastore, and runs a catalog operation that opens the
metastore; the test asserts that the child application succeeds.
**Why this works:** A child JVM gives the test sole control of
java.sql.DriverManager. The application-loader connection makes DriverManager
register only Spark's own Derby driver, which the Hive 4.1 client loader's
caller check rejects, so without the preload the client's first metastore
access fails with 'No suitable driver' and the child exits with an error. With
the preload, the client loader's own Derby driver is registered when the client
is constructed, and the catalog operation succeeds.
**Scope:** Test-only: one new regression test in the Hive spark-submit test
family plus the small child application it launches, added to the existing
suite file; no production change.
**Compatibility:** Production behavior is unchanged; the repair adds test
coverage only.
**Risks:** Adds one child-JVM run that resolves Hive 4.1 metastore jars from
Maven, so test time grows by about one existing Hive spark-submit test and
depends on the same artifact resolution the Hive client tests already need.
**Constraints:** The child application must open its Derby connection
through the application class loader before anything creates a Hive client, and
it must leave javax.jdo.option.ConnectionDriverName at its default, so the test
exercises the default local Derby setup the fix targets.
**Exclusions:**
- Coverage for local metastores whose driver-name setting does not match the
JDBC URL, which depends on the author's answer to the question about which
configurations the driver preload should cover.
- Correcting the SBT dedicated-JVM entry that names the HiveClientVersions
trait instead of a suite, which predates this change.
**Success:** Removing or mis-gating the driver preload makes the new test
fail with the child application's 'No suitable driver' error. With the preload
in place, the child application's catalog operation against its local Derby
metastore succeeds under both SBT and Maven, whichever suites ran earlier.
**Verification:**
- **Regression:** In a JVM where DriverManager was first used through the
application class loader, by an in-memory Derby connection opened before any
Hive client exists, an isolated Hive 4.1 client loaded from maven jars opens
its local Derby metastore and completes a catalog operation, and the same check
fails with 'No suitable driver' when the driver preload is removed.
- **Compatibility:** Existing coverage of isolated Hive 2.0 through 4.1
clients (and 4.2 on Java 21) against a local Derby metastore still passes, so
the new test changes no existing client behavior.
--
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]
