HIVE-29246: Upgrade derby version to 10.17.1.0 #6116

Aggarwal-Raghav · 2025-10-06T09:49:26Z

What changes were proposed in this pull request?

HIVE-29246, Upgrade derby version to 10.17.1.0

Derby project has done refactoring:

org.apache.derby.jdbc.EmbeddedDriver is now part of derbytools jar
org.apache.derby.security.SystemPermission → org.apache.derby.shared.common.security.SystemPermission (this is part of derbyshared jar which is compile time dependency for org.apache.derby:derby)
org.apache.derby.jdbc.AutoloadedDriver → org.apache.derby.iapi.jdbc.AutoloadedDriver

Why are the changes needed?

This will help with the CVE-2022-46337 and derby 10.17.1.0 uses JDK21, its inline with hive master branch

Does this PR introduce any user-facing change?

If users are using org.apache.derby.jdbc.EmbeddedDriver then they need to change the DriverName to org.apache.derby.iapi.jdbc.AutoloadedDriver

How was this patch tested?

Local setup testing. Will see CI outcome

Aggarwal-Raghav · 2025-10-08T19:47:14Z

CC @zabetak , if you have bandwidth can you please review this?

zabetak

The changes LGTM. One thing that I wanted to clarify is the distinction between derby and derbytools artifacts. Do we need both of them in the project? If the driver is now inside derbytools what does derby contain?

zabetak · 2025-10-09T08:48:25Z

...main/java/org/apache/hadoop/hive/metastore/dataconnector/jdbc/DerbySQLConnectorProvider.java

 public class DerbySQLConnectorProvider extends AbstractJDBCConnectorProvider {
  private static Logger LOG = LoggerFactory.getLogger(DerbySQLConnectorProvider.class);

  // private static final String DRIVER_CLASS = "org.apache.derby.jdbc.EmbeddedDriver".intern();


nit: Drop the comment

If other changes are needed for the PR then we can also tackle this nit otherwise let's leave it as is. Having a full precommit run just for deleting a comment isn't worth it.

zabetak · 2025-10-09T09:21:35Z

Following up on the question about derby and derbytools I am a bit unsure why the tools are needed. I was under the impression that the derby artifact has everything needed to run an embedded database.

Aggarwal-Raghav · 2025-10-09T09:23:25Z

The changes LGTM. One thing that I wanted to clarify is the distinction between derby and derbytools artifacts. Do we need both of them in the project? If the driver is now inside derbytools what does derby contain?

Thanks for the review @zabetak , Regarding this question.
I don't know the details as the commits in which this change is done is not very insightful :-)

But without this change the schema initialization fails as the https://github.com/apache/derby/blob/trunk/java/org.apache.derby.tools/org/apache/derby/jdbc/EmbeddedDriver.java looks for JDBCBoot class which is part of derby jar

(The screenshot is without derby jar in classpath, only derbytools and derbyshared are present)

Aggarwal-Raghav · 2025-10-09T09:29:47Z

Following up on the question about derby and derbytools I am a bit unsure why the tools are needed. I was under the impression that the derby artifact has everything needed to run an embedded database.

In short,
org.apache.derby.jdbc.EmbeddedDriver is part of => derbytools.jar
EmbeddedDriver needs JDBCBoot which is part of => derby.jar

zabetak · 2025-10-09T10:42:28Z

My interpretation of the Derby changes is that the derbytools is the optional part not the other way around. Did you try using Derby without adding derbytools in the classpath?

Aggarwal-Raghav · 2025-10-09T10:48:29Z

My interpretation of the Derby changes is that the derbytools is the optional part not the other way around. Did you try using Derby without adding derbytools in the classpath?

(derbytools jar is removed and derby, derbyshared are present)

zabetak · 2025-10-09T11:51:50Z

Explicitly loading a driver (using Class.forName) shouldn't be necessary in recent Java versions. Anyways, the code is there at the moment so probably this is outside of scope for the current PR. However, since the code wants to load a driver explicitly can we use AutoloadedDriver instead of the EmbeddedDriver by changing the appropriate Metastore conf?

Aggarwal-Raghav · 2025-10-09T12:05:27Z

Explicitly loading a driver (using Class.forName) shouldn't be necessary in recent Java versions. Anyways, the code is there at the moment so probably this is outside of scope for the current PR. However, since the code wants to load a driver explicitly can we use AutoloadedDriver instead of the EmbeddedDriver by changing the appropriate Metastore conf?

let me check on this and will update the PR/comment accordingly. Thanks for your input.

Aggarwal-Raghav · 2025-10-09T17:59:14Z

Explicitly loading a driver (using Class.forName) shouldn't be necessary in recent Java versions. Anyways, the code is there at the moment so probably this is outside of scope for the current PR. However, since the code wants to load a driver explicitly can we use AutoloadedDriver instead of the EmbeddedDriver by changing the appropriate Metastore conf?

So I tried using AutoloadedDriver in my setup (without derbytools) and it is working nicely. I also replaced all the EmbeddedDriver with AutoloadedDriver in code as well and it's compiling/building well.

I read about Service Provider Interface (SPI) and in (derby.jar) META-INF/services/java.sql.Driver contains org.apache.derby.iapi.jdbc.AutoloadedDriver and EmbeddedDriver is also using AutoloadedDriver internally.

https://github.com/apache/derby/blob/4253dcf4aa37dc64cf7235d494cd2f00f72e678a/java/org.apache.derby.tools/org/apache/derby/jdbc/EmbeddedDriver.java#L184

So, i think we can do the following:

Remove derbytools from all pom.xml
Replace org.apache.derby.jdbc.EmbeddedDriver -> org.apache.derby.iapi.jdbc.AutoloadedDriver everywhere
Remove Class.forName() from HiveSchemaHelper.java

Let me know your thoughts on this: Aggarwal-Raghav@70f740e Will update PR accordingly

zabetak · 2025-10-10T07:30:27Z

I think that Aggarwal-Raghav@70f740e is in the right direction. I am OK with items 1 and 2 but slightly unsure about 3 (removing Class.forName).

If possible let's tackle item 3 in a separate ticket/PR and do it in a more holistic way checking for other places where we load JDBC drivers explicitly. We should also verify that all supported drivers can be loaded automatically via the SPI mechanism.

Aggarwal-Raghav · 2025-10-10T09:01:23Z

I think that Aggarwal-Raghav@70f740e is in the right direction. I am OK with items 1 and 2 but slightly unsure about 3 (removing Class.forName).

If possible let's tackle item 3 in a separate ticket/PR and do it in a more holistic way checking for other places where we load JDBC drivers explicitly. We should also verify that all supported drivers can be loaded automatically via the SPI mechanism.

Sure, I have updated the PR and and updated the description "User-facing change" and filed HIVE-29258 for handling [3]

sonarqubecloud · 2025-10-10T10:07:14Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

zabetak

LGTM will merge soon! Please check if there is any of the doc in the Website that needs to be updated.

Aggarwal-Raghav · 2025-10-10T14:54:41Z

LGTM will merge soon! Please check if there is any of the doc in the Website that needs to be updated.

thanks for the review @zabetak . Yes there are few mention of EmbeddedDriver in Website. Will raise a pr for the same shortly.

Aggarwal-Raghav · 2025-10-10T17:51:32Z

Raised apache/hive-site#67.

Also, can we please add the following/similar line in the commit message:

HIVE-29246: Upgrade derby version to 10.17.1.0

* Migration from org.apache.derby.jdbc.EmbeddedDriver -> org.apache.derby.iapi.jdbc.AutoloadedDriver

asf-ci-hive added tests pending tests unstable and removed tests pending labels Oct 6, 2025

Aggarwal-Raghav force-pushed the HIVE-29246 branch from db0321f to 08617da Compare October 8, 2025 17:07

asf-ci-hive added tests pending and removed tests unstable labels Oct 8, 2025

Aggarwal-Raghav marked this pull request as ready for review October 8, 2025 19:46

asf-ci-hive added tests passed and removed tests pending labels Oct 8, 2025

zabetak reviewed Oct 9, 2025

View reviewed changes

Aggarwal-Raghav added 2 commits October 10, 2025 13:08

HIVE-29246: Upgrade derby version to 10.17.1.0

82fa864

Address review comments

3b73941

Aggarwal-Raghav force-pushed the HIVE-29246 branch from 08617da to 3b73941 Compare October 10, 2025 08:38

asf-ci-hive added tests pending and removed tests passed labels Oct 10, 2025

asf-ci-hive added tests passed and removed tests pending labels Oct 10, 2025

zabetak approved these changes Oct 10, 2025

View reviewed changes

zabetak merged commit 009dea2 into apache:master Oct 13, 2025
4 checks passed

HIVE-29246: Upgrade derby version to 10.17.1.0 #6116

HIVE-29246: Upgrade derby version to 10.17.1.0 #6116

Uh oh!

Conversation

Aggarwal-Raghav commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Aggarwal-Raghav commented Oct 8, 2025

Uh oh!

zabetak left a comment

Choose a reason for hiding this comment

Uh oh!

zabetak Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

zabetak commented Oct 9, 2025

Uh oh!

Aggarwal-Raghav commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Aggarwal-Raghav commented Oct 9, 2025

Uh oh!

zabetak commented Oct 9, 2025

Uh oh!

Aggarwal-Raghav commented Oct 9, 2025

Uh oh!

zabetak commented Oct 9, 2025

Uh oh!

Aggarwal-Raghav commented Oct 9, 2025

Uh oh!

Aggarwal-Raghav commented Oct 9, 2025

Uh oh!

zabetak commented Oct 10, 2025

Uh oh!

Aggarwal-Raghav commented Oct 10, 2025

Uh oh!

sonarqubecloud bot commented Oct 10, 2025

Quality Gate passed

Uh oh!

zabetak left a comment

Choose a reason for hiding this comment

Uh oh!

Aggarwal-Raghav commented Oct 10, 2025

Uh oh!

Aggarwal-Raghav commented Oct 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Aggarwal-Raghav commented Oct 6, 2025 •

edited

Loading

Aggarwal-Raghav commented Oct 9, 2025 •

edited

Loading