-
Notifications
You must be signed in to change notification settings - Fork 76
Added a fallback mechanism for the reading metadata #1588
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
Conversation
…MariaDB driver, improve fallback mechanisms for unsupported JDBC metadata methods, and update relevant tests.
|
@copilot make a review of fallback mechanism from the point of supporting some JDBC sources which are not supported yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances JDBC metadata reading robustness by adding comprehensive fallback mechanisms for drivers that don't fully implement JDBC metadata methods (e.g., Apache Hive), and fixes MariaDB-specific type mapping issues for BIGINT and unsigned integer types.
Key Changes:
- Added graceful fallbacks in
getTableColumnsMetadata()for all ResultSetMetaData methods that may throw SQLFeatureNotSupportedException - Fixed MariaDB type mappings to ensure BIGINT and unsigned integers always map to Long type, addressing driver inconsistencies
- Improved test robustness with proper resource cleanup using
.use {}blocks and consistent table naming
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DbType.kt |
Added comprehensive exception handling fallbacks for JDBC metadata retrieval methods (catalog, schema, column names, types, nullability) and updated documentation to reflect fallback behavior |
dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/MariaDb.kt |
Added explicit type mapping rules for BIGINT and unsigned integer types (INTEGER UNSIGNED, INT UNSIGNED) to consistently map to Long, fixing driver-reported type inconsistencies |
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/local/mariadbTest.kt |
Updated test assertion to expect ByteArray instead of Blob for TINYBLOB columns, aligning with corrected type mapping |
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/commonTestScenarios.kt |
Refactored test to use constant for table name, added proper cleanup with DROP TABLE IF EXISTS, and improved resource management with .use {} blocks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DbType.kt
Outdated
Show resolved
Hide resolved
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/commonTestScenarios.kt
Outdated
Show resolved
Hide resolved
dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/DbType.kt
Outdated
Show resolved
Hide resolved
dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/db/MariaDb.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Fixes #1220 + some discovered bugs regarding type mappings