Enhance Arrow metadata handling in JDBC driver for Thrift flow [Draft]#851
Closed
madhav-db wants to merge 1618 commits into
Closed
Enhance Arrow metadata handling in JDBC driver for Thrift flow [Draft]#851madhav-db wants to merge 1618 commits into
madhav-db wants to merge 1618 commits into
Conversation
* parser * more complex data type logic * complete complex data types * more tests * refine * integration test * add test to driver test * removed secret * fmt * fix unsupported test * integration test working * fmt * comments * fmt * update method sig * fmt * comments * comments * comments * fmt * fmt * add * more extensive testing * more extensive testing * more extensive testing * more extensive testing * more extensive testing
* Send intial connection log to unauth telemetry endpoint * Add tests + remove debug logs * fmt * Add tests * Add tests * Add tests for classes i didn't add:) * add getters/setters in systemConfiguration + improve coverage
- Run execution queries in `async` mode - Run metadata queries in `async` mode - Refactor result retrieval in Thrift by improving response handling and fixing polling - Refactor credential refreshing by transitioning from Thrift accessor to Thrift transport - Competing driver, SQL Gateway, and SQL Execution API also execute the queries in async mode - Minor code cleanup Running queries in async mode and using polling will be beneficial: - Freeing up server resources under high load instead of holding the connection for ~5 seconds (direct results) - If query takes more than 5 seconds, server will anyway require polling from client - Didn't notice any regressions (slightly better performance) under high concurrent load when running queries in async mode. Some empirical results below: - Running a simple select query on a table with 10-15 columns for 10K rows in a concurrent environment (250 connections executing query in parallel) - Average results when running on `main` ``` Average query time across connections: 73873.58ms Min query time: 22075ms Max query time: 88997ms ``` - Average results when running in `async` mode ``` Average query time across connections: 68382.95ms Min query time: 24718ms Max query time: 95282ms ```
…ch (#622) This adds a workflow to run JDBC comparator in a separate branch for GA readiness. Note that this does not affect the main branch.
Add databricks protected runners
Use databricks runners
Setup PAT correctly
…y paths (#620) * change file paths * change file paths * change file paths * change file paths * change file paths * change file paths * change file paths
Earlier it was PWD which is already set to some path!!
This is because we need the workflow to proceed further and email the differences. The test will instead send the output to STDERR.
Minor changes to github workflow around JDBC comparator
This reverts commit 5124293.
This will send a report around 00:00 UTC. The code is in the branch jdbc-comparator. The reports will be saved as artifacts of github workflow run.
* Added executeLargeUpdate * Added more unimplemented methods * Some more commands * Added more commands * Refractor * Added not implemented features * Added unit tests * nit * Refractored * Added unit tests * nit * NIT * added executeLargeBatch update * refractor
## Description - Retrieve all catalogs. - For each catalog, fetch schemas in parallel and merge the results. - On the e2-dogfood environment, which has around 9,000 schemas, this approach took approximately 12 seconds—compared to 9 seconds with the existing driver—an acceptable difference. - The runtime update for SHOW SCHEMAS IN ALL CATALOGS is currently under BEHAVE/compatibility review. Meanwhile, this client-side implementation serves as a temporary solution. - Introduced a JdbcThreadUtil class to simplify multi-threaded operations in JDBC. To ensure correctness, thread context must carry the appropriate connection-related details. ## Testing - End to end testing - Unit tests ## Additional Notes to the Reviewer - The BEHAVE committee reviewing [runtime PR #139818](https://github.com/databricks-eng/runtime/pull/139818) is expected to require the introduction of a SQL configuration. For JDBC clients to take advantage of these changes, users would need to manually set this SQL config in their Spark session. ~~Currently afaik, it's not possible to set this configuration directly through the JDBC connection.~~ We need to add a DBSQL config at https://github.com/databricks-eng/universe/blob/68a3f9bf4aa09b4d28f85229cbbb4c6e7bcef7e2/common/dbsql-config/src/SqlConfig.scala#L35 for the configuration which is WIP.
* NIT * nit * fake service tests
… (#822) * Added connection params to DatabricksDriverPropertyUtil * added connection params * params in driver property util Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * ssl related properties Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> --------- Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>
* init changes * update error message * update error message * added unit tests * return only body in toLiteral * edge case * handle edge case * add logging for edge case * correct getColumn() output for interval type * added fake service test * address comment * added a TODO * removed unused constants
* adds fallback for complex types * Add null check for requiredType in getArrowMetadataWithComplexTypeFallback method * Refactor ArrowStreamResult to simplify complex type handling and remove unused constants * undo * Enhance ArrowStreamResult with complex type handling and add unit tests for validation * Add complex type handling in DatabricksResultSet and improve exception messages for disabled support - Introduced a private method `isComplexType` to streamline complex type checks. - Updated exception messages in `getArray`, `getMap`, and `getStruct` methods to provide clearer guidance on enabling complex datatype support. - Added unit tests to verify exception handling when complex datatype support is disabled. * Refactor exception handling in DatabricksResultSet to use DatabricksSQLException - Updated methods getArray, getStruct, and getMap to throw DatabricksSQLException with specific error codes when complex datatype support is disabled. - Enhanced logging for error conditions to improve debugging and user guidance. - Adjusted unit tests to verify the new exception types and error codes. * - Added a logger to ArrowStreamResult to provide debug information when complex datatype support is disabled. - Improved clarity in handling complex types by logging the conversion to STRING when necessary.
…es when param is disabled (string handling) (#839)
* Enhance ComplexDataTypeParser with formatting methods for complex types
- Added `formatComplexTypeString` method to standardize JSON string representation for complex types when support is disabled.
- Introduced `formatMapString` method to convert map JSON strings into a consistent {key:value} format.
- Updated `ArrowStreamResult` to utilize the new formatting methods for handling complex types when complex datatype support is disabled.
* unit test
* Refactor formatMapString method in ComplexDataTypeParser to improve map metadata handling
- Changed the initialization of the key-value array in the formatMapString method to use a dynamic array instead of a fixed size.
- Removed commented-out code for clarity and improved readability.
* fix
* addresses comments
* Release 1.0.6-oss --------- Co-authored-by: Jayant Singh <jayant.singh@databricks.com>
- Add nvdApiKey configuration to OWASP dependency-check-maven plugin - Configure retry settings and API delay for better performance - Update GitHub Actions workflow to pass NVD_API_KEY secret - Resolves warning about missing NVD API key and improves scan performance Without this, release shows a warning: An NVD API Key was not provided - it is highly recommended to use an NVD API key as the update can take a VERY long time without an API Key
- Add `Support for INTERVAL data type.` - This was not added to NEXT_CHANGELOG by authors
… Query (#838) * Basic working * Added tests * nit * nit * PR comments * nit * Added e2e tests * Removed Describe query logs * log fix * Updated error handling
* Update SDK version2 * Fix test
* fixed actions * nit * nit * nit * check * nit
* added application name to track BiTool * add tests * some changes * refactored code; update user agent and telemetry client app * update user agent thrift mode as well * changes fn to package-private. * make parameterized test * spotless * merge UserAgentHelper and UserAgentManager * addressed reviews * update DatabricksHttpClient
* Add statement ID and session ID wherever possible * Add null check tests
* ADded logging * Added logging
* Add support for SSL client certificate authentication - Implemented client certificate authentication via keystore configuration parameters. - Updated `ConfiguratorUtils` to handle loading and managing keystores. - Enhanced unit tests to cover keystore loading and client certificate scenarios. * - Removed redundant null check for the keystore in createKeyManagers method. - Simplified key entry verification by replacing Enumeration with a stream-based approach for better readability and performance. * addresses comments * address comments * fix next changelog * Remove unnecessary copy command for intermediateCA.crt in sslTesting workflow * Update sslTesting workflow to correct client certificate configuration for mutual TLS * Update sslTesting workflow to include intermediate CA certificate and remove obsolete DNS directive * revert to main * Refactor HTTP exception handling to use DatabricksSSLException for SSL/TLS errors. Update relevant classes and tests to replace DatabricksHttpException with DatabricksSSLException, ensuring proper error handling during SSL configuration and handshake processes. * Update error message in DatabricksClientConfiguratorManager to reflect SSL error handling instead of HTTP error. This change enhances clarity in logging and exception management related to SSL configuration failures.
- Implemented logic to utilize Arrow metadata for column types in DatabricksResultSetMetaData. - Updated ArrowResultChunk and related classes to pass and manage Arrow metadata effectively. - Modified InlineChunkProvider and RemoteChunkProvider to accept Arrow metadata during initialization. - Added extraction of Arrow metadata from schema in ArrowStreamResult. - Updated tests to accommodate changes in metadata handling across various components.
|
Please ensure that the NEXT_CHANGELOG.md file is updated with any relevant changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Testing
Additional Notes to the Reviewer