Skip to content

feat(flightsql-jdbc): Add support for Arrow Flight JDBC driver (#8829) #9789

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vincentditlevinz
Copy link

@vincentditlevinz vincentditlevinz commented Jul 17, 2025

Check List

  • Tests have been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

[#8829]

@vincentditlevinz vincentditlevinz requested review from a team as code owners July 17, 2025 10:55
@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Jul 17, 2025
@vincentditlevinz
Copy link
Author

vincentditlevinz commented Jul 17, 2025

Sorry for the level of testing, as I said I've followed packages/cubejs-databricks-jdbc-driver as a template and did not find a lot of tests inside, same with the jdbc drivers module. I might try something with testcontainer...

That said, I successfully managed to build a dev docker image and connect to a spice.ai sql engine supporting arrow flight sql jdbc driver using CUBEJS_DB_TYPE=flightsql-jdbc.

The driver download works as shown in this screenshot showing the dev container internal file system:

image

However I cannot do more than connecting to the database when clicking on the "Data model" folder.

image

I might have missed something or perhaps a dev container is not full featured ? When I try to see the generated SQL in the playground I see Error: Unknown dbType: flightsql-jdbc, and in container's logs:

image

@vincentditlevinz vincentditlevinz force-pushed the support-for-arrow-flight-jdbc-driver branch 8 times, most recently from 3183c70 to 2db653b Compare July 17, 2025 15:06
@igorlukanin
Copy link
Member

Hi @vincentditlevinz 👋 Thanks! I'd suggest that you can you databricks-jdbc as an example and check if there's anything else that needs to be updated: https://github.com/search?q=repo%3Acube-js%2Fcube%20databricks-jdbc&type=code

@vincentditlevinz vincentditlevinz force-pushed the support-for-arrow-flight-jdbc-driver branch from 2db653b to 2f90fc9 Compare July 18, 2025 12:17
@vincentditlevinz
Copy link
Author

vincentditlevinz commented Jul 18, 2025

High @igorlukanin

I managed to add some tests with testcontainer that have worked on my laptop 😄 They test testConnection() and a simple query (count(*)) against an Arrow flight SQL compatible database.

When I compared all occurrences of databrick-jdbc with my PR I found that I haven't yet:

  • written any doc.
  • written anything in cubejs-testing/birdbox-fixture, not sure I should.
  • written anything in cubejs-testing-drivers. It sounds a bit complex to me to understand what I should code there.
  • Add a line in cubejs-docker/package.json, because it won't work until we first deploy flightsql-jdbc driver in the NPM repository (same for the CLI that won't work until this moment too). As far as I understand, a new driver can only be deployed in a "latest" docker image if it has been already deployed in the NPM repository.

The last item probably explains why my new driver doesn't work well in the playground, because I must use a "dev" docker image for the moment.

Please, let me know what you expect me to do to make this PR valid according to your development standards.

@vincentditlevinz vincentditlevinz force-pushed the support-for-arrow-flight-jdbc-driver branch from 2f90fc9 to 2c36eb1 Compare July 18, 2025 13:08
@@ -20,6 +20,7 @@ on:
- 'packages/cubejs-bigquery-driver/**'
- 'packages/cubejs-clickhouse-driver/**'
- 'packages/cubejs-databricks-jdbc-driver/**'
- 'packages/cubejs-flightsql-jdbc-driver/**'
Copy link
Author

Choose a reason for hiding this comment

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

I think I should remove these lines, they seem to be related to cubejs-testing-driver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants