feat: Oracle connector enhancements by Dilli-Babu-Godari · Pull Request #25355 · prestodb/presto

Dilli-Babu-Godari · 2025-06-18T09:57:05Z

Description

Oracle connector enhancements.

Motivation and Context

Impact

Test Plan

presto> CREATE TABLE oracle.tm_lakehouse_engine.orcaemp2 (
     ->     empid DOUBLE,
     ->     empname VARCHAR(255),
     ->     empbandcode DOUBLE,
     ->     location DOUBLE,
     ->     empdepcode DOUBLE,
     ->     address VARCHAR(255),
     ->     email VARCHAR(255),
     ->     ssn VARCHAR(255),
     ->     phone VARCHAR(255)
     -> );
CREATE TABLE

Query 20260326_071832_00019_85sxf, FINISHED, 0 nodes
http://localhost:8080/ui/query.html?20260326_071832_00019_85sxf
Splits: 0 total, 0 done (0.00%)
[Latency: client-side: 0:12, server-side: 0:12] [0 rows, 0B] [0 rows/s, 0B/s]


presto> INSERT INTO oracle.tm_lakehouse_engine.orcaemp2
     ->     (empid, empname, empbandcode, location, empdepcode, address, email, ssn, phone)
     -> VALUES
     ->     (1.0, 'John Doe', 3.0, 101.0, 20.0, '123 Main St, New York, NY', 'john.doe@example.com', '123-45-6789', '555-123-4567');

Query 20260326_071911_00020_85sxf, RUNNING, 1 node, 35 splits
http://localhost:8080/ui/query.html?20260326_071911_00020_85sxf

INSERT: 1 row

Query 20260326_071911_00020_85sxf, FINISHED, 1 node
http://localhost:8080/ui/query.html?20260326_071911_00020_85sxf
Splits: 35 total, 35 done (100.00%)
[Latency: client-side: 0:44, server-side: 0:44] [0 rows, 0B] [0 rows/s, 0B/s]


presto> SELECT * FROM oracle.tm_lakehouse_engine.orcaemp2;

Query 20260326_072023_00021_85sxf, RUNNING, 1 node, 17 splits

empid | empname  | empbandcode | location | empdepcode |          address          |        email         |     ssn     |    phone     
-------+----------+-------------+----------+------------+---------------------------+----------------------+-------------+--------------
   1.0 | John Doe |         3.0 |    101.0 |       20.0 | 123 Main St, New York, NY | john.doe@example.com | 123-45-6789 | 555-123-4567 
(1 row)

Query 20260326_072023_00021_85sxf, FINISHED, 1 node
http://localhost:8080/ui/query.html?20260326_072023_00021_85sxf
Splits: 17 total, 17 done (100.00%)
[Latency: client-side: 0:21, server-side: 0:21] [1 rows, 0B] [0 rows/s, 0B/s]

Testing for fetch_size:

Presto-CLI:

jdbc-fetch-size = 10

presto> select * from oracle_small_fetch.TM_LAKEHOUSE_ENGINE.TEST_FETCH;
    ID    |  DATA   
------------------+------------
 1211.0000000000 | data_1211  
 1212.0000000000 | data_1212  
 1213.0000000000 | data_1213  
 1214.0000000000 | data_1214  
 1215.0000000000 | data_1215  
 1216.0000000000 | data_1216  
 1217.0000000000 | data_1217  
 1218.0000000000 | data_1218  
 1219.0000000000 | data_1219  
 1220.0000000000 | data_1220
(query aborted by user)

Query 20260407_144910_00006_m2vrb, RUNNING, 1 node
Splits: 17 total, 0 done (0.00%)
[Latency: client-side: 1:15, server-side: 1:11] [0 rows, 0B] [0 rows/s, 0B/s]

20260407_144910_00006_m2vrb_small_fetch.json

jdbc-fetch-size = 1000

presto> select * from oracle_large_fetch.TM_LAKEHOUSE_ENGINE.TEST_FETCH;
    ID    |  DATA   
------------------+------------
 1211.0000000000 | data_1211  
 1212.0000000000 | data_1212  
 1213.0000000000 | data_1213  
 1214.0000000000 | data_1214  
 1215.0000000000 | data_1215  
 1216.0000000000 | data_1216  
 1217.0000000000 | data_1217  
 1218.0000000000 | data_1218  
 1219.0000000000 | data_1219  
 1220.0000000000 | data_1220  
 1221.0000000000 | data_1221  
 1222.0000000000 | data_1222
(query aborted by user)

Query 20260407_144746_00005_m2vrb, RUNNING, 1 node
Splits: 17 total, 0 done (0.00%)
[Latency: client-side: 0:11, server-side: 0:06] [0 rows, 0B] [0 rows/s, 0B/s]

20260407_144746_00005_m2vrb_large_fetch.json

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Oracle Connector Changes
*  Add oraorai18n.jar dependency, jdbc fetchsize and SSL config for Oracle connector.

linux-foundation-easycla · 2025-06-18T09:57:09Z

The committers listed above are authorized under a signed CLA.

✅ login: Dilli-Babu-Godari / name: Dilli-Babu-Godari (1e667d1, 62252de, 7e4b878, c1999ef, c2f463d)

steveburnett · 2025-06-18T13:55:11Z

Thanks for the release note entry!

The PR is now automatically included, you don't have to add it in yourself anymore.
Revise the entry to begin with a word from the Order of changes in the Release Notes Guidelines: is the work in this PR a Fix, for example?
Expand the abbreviation WxD.
Should the section be Oracle Connector Changes instead of General Changes?

prestodb-ci · 2025-06-19T07:56:26Z

@ethanyzhang imported this issue as lakehouse/presto #25355

agrawalreetika

I have a few comments -

Let's split PR in 2 commits - TLS & fetchsize changes
I think TLS is going to be a generic feature acorss different JDBC connectors, maybe we can create a separate TLS config or have it in BaseJDBC config and reuse it in the required JDBC connector?

presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleConfig.java

Dilli-Babu-Godari · 2025-06-20T04:18:29Z

I have a few comments -

Let's split PR in 2 commits - TLS & fetchsize changes

I think TLS is going to be a generic feature acorss different JDBC connectors, maybe we can create a separate TLS config or have it in BaseJDBC config and reuse it in the required JDBC connector?

This PR currently includes 4 combined PR changes. I can split them into 2 separate commits as you suggested.

Dilli-Babu-Godari · 2025-06-20T09:37:17Z

Thanks for the release note entry!

The PR is now automatically included, you don't have to add it in yourself anymore.

Revise the entry to begin with a word from the Order of changes in the Release Notes Guidelines: is the work in this PR a Fix, for example?

Expand the abbreviation WxD.

Should the section be Oracle Connector Changes instead of General Changes?

Updated the changes

presto-oracle/pom.xml

steveburnett · 2025-06-20T13:43:58Z

Updated the changes

Thanks for addressing the last point! Please address these:

The PR is now automatically included, you don't have to add it in yourself anymore.
Revise the entry to begin with a word from the Order of changes in the Release Notes Guidelines: is the work in this PR a Fix, for example?
Expand the abbreviation WxD.

Dilli-Babu-Godari · 2025-07-28T05:27:55Z

Updated the changes

Thanks for addressing the last point! Please address these:

The PR is now automatically included, you don't have to add it in yourself anymore.

Revise the entry to begin with a word from the Order of changes in the Release Notes Guidelines: is the work in this PR a Fix, for example?

Expand the abbreviation WxD.

Updated the changes.

prestodb-ci · 2026-03-19T14:00:30Z

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

prestodb-ci · 2026-03-19T14:00:31Z

@faizdani
Can you please provide an update on this issue? Thank you!

Dilli-Babu-Godari · 2026-03-26T09:18:39Z

This PR depends on the test-enabling PR: #25762 for adding tests.

prestodb-ci · 2026-03-26T14:00:04Z

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

prestodb-ci · 2026-03-26T14:00:05Z

@faizdani
Can you please provide an update on this issue? Thank you!

github-actions · 2026-03-31T07:22:06Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 1968770...5310db8.

No notifications.

agrawalreetika

Thanks for the PR.
Please add documentation for all the newly added properties.

agrawalreetika · 2026-04-01T06:25:29Z

presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcConfig.java

+    {
+        this.fetchSize = fetchSize;
+        return this;
+    }


Is this applicable for other JDBC connectors as well? If yes, then we need to make usagae of it in other connectors as well

Yes, the jdbc-fetch-size configuration in BaseJdbcConfig is designed to be applicable across all JDBC connectors. We should make the changes in connectors that support.

Then it would be good to either open a follow up PR or an issue

Yes, I’ll create an issue to track this and follow up with a PR.

Can you link the issue to this, if you have it?

presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcConfig.java

agrawalreetika · 2026-04-01T07:15:30Z

presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleClient.java

                escapeNamePattern(schemaName, Optional.of(escape)).orElse(null),
                escapeNamePattern(tableName, Optional.of(escape)).orElse(null),
                getTableTypes());
+        resultSet.setFetchSize(fetchSize);


This doesn't look correct - setFetchSize() is called on the ResultSet after metadata.getTables() has already executed.

I see OracleConnection has setDefaultRowPrefetch, so it should be set in the OracleConnection https://github.com/mikesmithjr/ora-jdbc-source/blob/master/OracleJDBC/src/oracle/jdbc/OracleConnection.java#L97

Thanks for pointing this out.

Updated the implementation to unwrap OracleConnection and set setDefaultRowPrefetch(fetchSize) before query execution. Applied this consistently in both getTables() and getPreparedStatement().

Can you help me understand how it is setting it in getTables() and getPreparedStatement(), the same as setting it in ConnectionFactory so it can be applied across all the operatoins?

Oracle connector currently uses the default DriverConnectionFactory, so introducing a connection-level setting would require adding a custom factory.

Since this optimization is primarily relevant for specific operations like getTables() and prepared statements, scoped it at the method level, where large result set fetching is expected.

This avoids introducing connector-wide behavior changes while keeping the optimization targeted. That said, we can move this to a custom OracleConnectionFactory if we want it to apply uniformly across all operations.

In my opinion, it would be better to keep the fetch consistent across since it's an Oracle batch fetch-related config. Open to hear suggestions as well

I will introduce a custom OracleConnectionFactory for the batch fetch-related config instead of doing it in the connectonFactory in OracleClientModule.java

presto/presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleClientModule.java

Line 54 in ee0eec7

return new DriverConnectionFactory(

what do you think of this ?

presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleConfig.java

presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcConfig.java

steveburnett

A local doc build returns the following error:

/Users/steveburnett/Documents/GitHub/presto/presto-docs/src/main/sphinx/connector/oracle.rst:56: ERROR: Malformed table. Bottom/header table border does not match top border.

The entire table in General Configuration Properties is not displayed as a result. See screenshot of my local doc build.

Please fix the formatting of the table and re-request my review, and I'll be happy to review the content.

Dilli-Babu-Godari · 2026-04-01T13:50:38Z

A local doc build returns the following error:

/Users/steveburnett/Documents/GitHub/presto/presto-docs/src/main/sphinx/connector/oracle.rst:56: ERROR: Malformed table. Bottom/header table border does not match top border.

The entire table in General Configuration Properties is not displayed as a result. See screenshot of my local doc build.
Please fix the formatting of the table and re-request my review, and I'll be happy to review the content.

Thanks fixed alignment issues.

prestodb-ci · 2026-04-02T14:00:01Z

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

prestodb-ci · 2026-04-02T14:00:02Z

@faizdani
Can you please provide an update on this issue? Thank you!

steveburnett

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

Oracle connector fails with a connection timeout, while show tables, when the schema has a 1 lakh tables. This happens because we are not specifying the fetch_size with the getTables method in Oracle. As a result, the connector uses the default fetch_size, which is a smaller value (10), leading to multiple network calls to the datasource and ultimately causing a connection timeout., Co-authored-by: lukmanulhakkeem <lukmanul.hakkeem.a@ibm.com> # Conflicts: # presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleClient.java

added the orai18n.jar dependency, ensuring compatibility with non-UTF character sets like WE8ISO8859P9.

SSL enablement of Oracle Connector with Oracle THIN JDBC driver Co-authored-by: lukmanulhakkeem <lukmanul.hakkeem.a@ibm.com>

agrawalreetika

Mostly lgtm apart from few comments.

agrawalreetika · 2026-04-08T06:19:43Z

presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcConfig.java

+    {
+        this.fetchSize = fetchSize;
+        return this;
+    }


Can you link the issue to this, if you have it?

agrawalreetika · 2026-04-08T06:22:02Z

presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleClientModule.java

+                connectionProperties.setProperty("javax.net.ssl.trustStorePassword", oracleConfig.getTruststorePassword());
+            }
+        }
+        connectionProperties.setProperty(OracleConnection.CONNECTION_PROPERTY_INCLUDE_SYNONYMS, String.valueOf(oracleConfig.isSynonymsEnabled()));


use static import

agrawalreetika · 2026-04-08T06:27:07Z

presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleClientModule.java

+        if (oracleConfig.isTlsEnabled()) {
+            if (oracleConfig.getTrustStorePath() != null) {
+                connectionProperties.setProperty("javax.net.ssl.trustStore", oracleConfig.getTrustStorePath());
+            }
+            if (oracleConfig.getTruststorePassword() != null) {
+                connectionProperties.setProperty("javax.net.ssl.trustStorePassword", oracleConfig.getTruststorePassword());
+            }
+        }


Suggested change

if (oracleConfig.isTlsEnabled()) {

if (oracleConfig.getTrustStorePath() != null) {

connectionProperties.setProperty("javax.net.ssl.trustStore", oracleConfig.getTrustStorePath());

}

if (oracleConfig.getTruststorePassword() != null) {

connectionProperties.setProperty("javax.net.ssl.trustStorePassword", oracleConfig.getTruststorePassword());

}

}

if (oracleConfig.isTlsEnabled()) {

requireNonNull(oracleConfig.getTrustStorePath(), "oracle.tls.truststore-path is null");

connectionProperties.setProperty("javax.net.ssl.trustStore", oracleConfig.getTrustStorePath());

connectionProperties.setProperty("javax.net.ssl.trustStorePassword", oracleConfig.getTruststorePassword());

}

We can simplify this. Since the truststore password can be null for self-signed certs, null checks may not be needed for this.

Dilli-Babu-Godari requested a review from a team as a code owner June 18, 2025 09:57

Dilli-Babu-Godari requested a review from hantangwangd June 18, 2025 09:57

prestodb-ci added the from:IBM PR from IBM label Jun 18, 2025

prestodb-ci requested review from a team, agrawalreetika and infvg and removed request for a team June 18, 2025 09:57

Dilli-Babu-Godari force-pushed the oracle_oss_pr branch 2 times, most recently from 0e2d96e to cf1e277 Compare June 18, 2025 10:12

Dilli-Babu-Godari changed the title ~~Oracle connector enhancements in WxD~~ Oracle connector enhancements Jun 18, 2025

agrawalreetika reviewed Jun 19, 2025

View reviewed changes

presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleConfig.java Outdated Show resolved Hide resolved

presto-oracle/src/main/java/com/facebook/presto/plugin/oracle/OracleConfig.java Outdated Show resolved Hide resolved

Dilli-Babu-Godari force-pushed the oracle_oss_pr branch 2 times, most recently from 4830ea8 to e15a068 Compare June 20, 2025 09:27

Dilli-Babu-Godari force-pushed the oracle_oss_pr branch 5 times, most recently from c378b25 to 6de476f Compare June 20, 2025 10:00

agrawalreetika reviewed Jun 20, 2025

View reviewed changes

presto-oracle/pom.xml Show resolved Hide resolved

Dilli-Babu-Godari force-pushed the oracle_oss_pr branch 4 times, most recently from 564d0ea to beaac68 Compare July 29, 2025 07:38

prestodb-ci added follow-up-1 1st time follow-up (alchemy generated) need-follow-up Need any type of follow-up (alchemy generated) labels Mar 12, 2026

prestodb-ci added follow-up-2 2nd time follow-up (alchemy generated) and removed follow-up-1 1st time follow-up (alchemy generated) labels Mar 19, 2026

Dilli-Babu-Godari force-pushed the oracle_oss_pr branch from 8d8e7df to c6ad416 Compare March 26, 2026 07:26

Dilli-Babu-Godari changed the title ~~Oracle connector enhancements~~ feat: Oracle connector enhancements Mar 26, 2026

Dilli-Babu-Godari changed the title ~~feat: Oracle connector enhancements~~ feat: Oracle connector enhancements Mar 26, 2026

prestodb-ci added follow-up-3 3rd time follow-up (alchemy generated) and removed follow-up-2 2nd time follow-up (alchemy generated) labels Mar 26, 2026

Dilli-Babu-Godari force-pushed the oracle_oss_pr branch 3 times, most recently from 0334241 to 676b9fd Compare March 27, 2026 07:47

agrawalreetika requested changes Apr 1, 2026

View reviewed changes

steveburnett requested changes Apr 1, 2026

View reviewed changes

steveburnett previously approved these changes Apr 2, 2026

View reviewed changes

Dilli-Babu-Godari and others added 4 commits April 6, 2026 23:18

Adds orai18n.jar to ensure Oracle works with charsets like WE8ISO8859P9.

1e667d1

added the orai18n.jar dependency, ensuring compatibility with non-UTF character sets like WE8ISO8859P9.

Added ssl configs for oracle

62252de

SSL enablement of Oracle Connector with Oracle THIN JDBC driver Co-authored-by: lukmanulhakkeem <lukmanul.hakkeem.a@ibm.com>

Add TLS documentation for Oracle connector

7e4b878

agrawalreetika requested changes Apr 8, 2026

View reviewed changes

Implement custom ConnectionFactory to set Oracle's default row prefetch

c1999ef

Conversation

Dilli-Babu-Godari commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

linux-foundation-easycla bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steveburnett commented Jun 18, 2025

Uh oh!

prestodb-ci commented Jun 19, 2025

Uh oh!

agrawalreetika left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Dilli-Babu-Godari commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dilli-Babu-Godari commented Jun 20, 2025

Uh oh!

Uh oh!

steveburnett commented Jun 20, 2025

Uh oh!

Dilli-Babu-Godari commented Jul 28, 2025

Uh oh!

prestodb-ci commented Mar 19, 2026

Uh oh!

prestodb-ci commented Mar 19, 2026

Uh oh!

Dilli-Babu-Godari commented Mar 26, 2026

Uh oh!

prestodb-ci commented Mar 26, 2026

Uh oh!

prestodb-ci commented Mar 26, 2026

Uh oh!

github-actions bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agrawalreetika left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dilli-Babu-Godari Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

steveburnett left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dilli-Babu-Godari commented Jun 18, 2025 •

edited

Loading

linux-foundation-easycla bot commented Jun 18, 2025 •

edited

Loading

agrawalreetika left a comment •

edited

Loading

Dilli-Babu-Godari commented Jun 20, 2025 •

edited

Loading

github-actions bot commented Mar 31, 2026 •

edited

Loading

Dilli-Babu-Godari Apr 6, 2026 •

edited

Loading

steveburnett left a comment •

edited

Loading