uros-b commented on code in PR #58766:
URL: https://github.com/apache/spark/pull/58766#discussion_r4001300102


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala:
##########
@@ -84,6 +84,12 @@ class JDBCOptions(
   require(parameters.isDefinedAt(JDBC_URL), s"Option '$JDBC_URL' is required.")
   // a JDBC URL
   val url = parameters(JDBC_URL)
+  // `isDefinedAt` above only checks the key is present; the value can still 
be `null`. Guard it
+  // here so a null url surfaces as a user-facing error instead of a raw 
NullPointerException from
+  // `DriverManager.getDriver(url)` below. Mirrors the null-value guard in 
`asProperties`.
+  if (url == null) {

Review Comment:
   @dejankrak-db Could you just please check whether this is reachable:
   
   The new url == null check never runs. asProperties is initialized first 
(SPARK-48601) and already throws NULL_DATA_SOURCE_OPTION for every null option, 
including url. The new test should pass on master without the JDBCOptions 
change.



-- 
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]

Reply via email to