Skip to content

Conversation

@JoshRosen
Copy link
Contributor

What changes were proposed in this pull request?

This PR updates unit tests and docs to update references to long-deprecated Hive JDBC client connection string parameters.

The current docs and some test code are using parameters that have been deprecated for nearly a decade since https://issues.apache.org/jira/browse/HIVE-6972 / apache/hive@07082e8 , so I think it's safe to clean up the usages now.

Why are the changes needed?

While looking at some hive-thriftserver unit tests logs, I saw repeated spam of

20/06/05 06:35:55.442 pool-1-thread-1 WARN Utils: ***** JDBC param deprecation *****
20/06/05 06:35:55.442 pool-1-thread-1 WARN Utils: The use of hive.server2.transport.mode is deprecated.
20/06/05 06:35:55.442 pool-1-thread-1 WARN Utils: Please use transportMode like so: jdbc:hive2://<host>:<port>/dbName;transportMode=<transport_mode_value>
20/06/05 06:35:55.442 pool-1-thread-1 WARN Utils: ***** JDBC param deprecation *****
20/06/05 06:35:55.442 pool-1-thread-1 WARN Utils: The use of hive.server2.thrift.http.path is deprecated.
20/06/05 06:35:55.442 pool-1-thread-1 WARN Utils: Please use httpPath like so: jdbc:hive2://<host>:<port>/dbName;httpPath=<http_path_value> 

Does this PR introduce any user-facing change?

No, it's just a test + documentation change, recommending syntax which has been long-supported (some tests were already using the new parameters).

How was this patch tested?

Existing tests (let's wait to confirm that they pass in CI).

Was this patch authored or co-authored using generative AI tooling?

No.

Copy link
Contributor Author

@JoshRosen JoshRosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, looks like tests are failing:

### Attempt 0 ###
HiveThriftServer2 command line: ArraySeq(../../sbin/start-thriftserver.sh, --master, local, --hiveconf, javax.jdo.option.ConnectionURL=jdbc:derby:;databaseName=/home/runner/work/spark/spark/target/tmp/spark-dcc32478-081f-40f6-971e-270bd8654e1a;create=true, --hiveconf, hive.metastore.warehouse.dir=/home/runner/work/spark/spark/target/tmp/spark-0a92cbf9-1210-474c-a740-a1194d272...

[info] - SPARK-17819: Support default database in connection URIs *** FAILED *** (152 milliseconds)
[info]   java.sql.SQLException: Could not open client transport with JDBC Uri: jdbc:hive2://localhost:41681/default?transportMode=http;httpPath=cliservice;a=avalue;b=bvalue#c=cvalue;d=dvalue: Invalid status 72
[info]   at org.apache.hive.jdbc.HiveConnection.<init>(HiveConnection.java:224)
[info]   at org.apache.hive.jdbc.HiveDriver.connect(HiveDriver.java:107)
[info]   at java.sql/java.sql.DriverManager.getConnection(DriverManager.java:681)
[info]   at java.sql/java.sql.DriverManager.getConnection(DriverManager.java:229)
[info]   at org.apache.spark.sql.hive.thriftserver.HiveThriftServer2TestBase.$anonfun$withDatabase$2(HiveThriftServer2Suites.scala:1475)
[info]   at scala.collection.immutable.ArraySeq.map(ArraySeq.scala:75)
[info]   at scala.collection.immutable.ArraySeq.map(ArraySeq.scala:35)
[...]

@JoshRosen
Copy link
Contributor Author

I think the issue might be a ? querystring style parameter vs a ; as the delimiter separating the /database part from the other parameters; I've tried updating this and let's see if tests pass with that change.

@JoshRosen
Copy link
Contributor Author

I think I found the problem: the ? is a separator between JDBC session confs and hive confs: previously, hive.server2.transport.mode and hive.server2.thrift.http.path were being passed as Hive confs but in the updated examples I'm passing them as JDBC confs so I have to update the separator accordingly while still keeping the ? before the hive confs section.

@MaxGekk
Copy link
Member

MaxGekk commented Sep 17, 2024

+1, LGTM. Merging to master.
Thank you, @JoshRosen and @HyukjinKwon for review.

@MaxGekk MaxGekk closed this in dd8d127 Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants