Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDBC Engine supports Oracle #6815

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

naive-zhang
Copy link

🔍 Description

Issue References 🔗

This pull request fixes #

Describe Your Solution 🔧

Currently, Kyuubi supports JDBC engines with limited dialects, and I extend the dialects to support Oracle.

  • Introduce Oracle support in JDBC Engine
  • Adding dialects and tests for Oracle

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Supported Oracle in JDBC engine.

Related Unit Tests

Add tests of OperationWithOracleEngineSuite, OracleOperationSuite, OracleSessionSuite and OracleStatementSuite.


Checklist 📝

Be nice. Be informative.

@codecov-commenter
Copy link

codecov-commenter commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (c391d16) to head (abf9837).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala 0.00% 3 Missing ⚠️
...g/apache/kyuubi/jdbc/hive/KyuubiBaseResultSet.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #6815   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         687     687           
  Lines       42442   42446    +4     
  Branches     5793    5795    +2     
======================================
- Misses      42442   42446    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@@ -338,6 +338,10 @@ private Object evaluate(TTypeId columnType, Object value) {
if (value instanceof String) {
return ((String) value).getBytes();
}
case INT_TYPE:
if (value instanceof String) {
return Integer.valueOf((String) value);
Copy link
Member

Choose a reason for hiding this comment

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

why this is required?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your review, because in case of NUMBER(x,0) refering to Integer in Oracle, OJDBC returns it as java.sql.Types.NUMERIC. In this case, I implement toTTypeDesc method of OracleSchemaHelper to make sure NUMBER(x,0) treated as Integer in resultSet.getMetaData, but in evaluate method of KyuubiBaseResultSet, it treated java.sql.Types.NUMERIC as decimal, which is instance of String. Is my implementation OK?


import org.apache.kyuubi.engine.jdbc.connection.JdbcConnectionProvider

class OracleConnectionProvider extends JdbcConnectionProvider {
Copy link
Member

Choose a reason for hiding this comment

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

please update the docs for configuration kyuubi.engine.jdbc.connection.provider

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your review, and I've update the docs~

tableTypes.asScala.toSet
}
val query = new StringBuilder(
s"""SELECT OWNER AS TABLE_SCHEMA,
Copy link
Member

Choose a reason for hiding this comment

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

weired spaces

Copy link
Author

Choose a reason for hiding this comment

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

Well, I've already adjusted the wired space

@github-actions github-actions bot added kind:documentation Documentation is a feature! module:common labels Nov 19, 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.

4 participants