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.

Sign up for GitHub

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

Jump to bottom

Relational data ingestion via ADBC/ODBC #3279

Merged
merged 7 commits into from
Jan 31, 2023
Merged

Relational data ingestion via ADBC/ODBC #3279

merged 7 commits into from
Jan 31, 2023

Conversation

jmao-denver
Copy link
Contributor

@jmao-denver jmao-denver commented Jan 9, 2023 •
edited
Loading

  1. tightly coupled with Turbodbc
  2. requires users to learn Turbodbc in addition to ODBC configuration and setup
  3. requires DH's arrow module

Fixes #3131

@jmao-denver jmao-denver added this to the Jan 2023 milestone Jan 9, 2023
@jmao-denver jmao-denver requested a review from chipkent January 9, 2023 19:29
@jmao-denver jmao-denver self-assigned this Jan 9, 2023
@jmao-denver jmao-denver force-pushed the 3131-odbc-integration branch from a8c79be to d33cc6d Compare January 11, 2023 15:26
@jmao-denver jmao-denver changed the title 3131 odbc integration Relational data ingestion via ODBC Jan 11, 2023
@jmao-denver jmao-denver force-pushed the 3131-odbc-integration branch from d33cc6d to 8b64035 Compare January 11, 2023 23:05
chipkent
chipkent requested changes Jan 13, 2023
View reviewed changes
py/server/deephaven/dbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc.py Outdated Show resolved Hide resolved
@jmao-denver jmao-denver requested a review from chipkent January 17, 2023 15:16
chipkent
chipkent requested changes Jan 17, 2023
View reviewed changes
py/server/deephaven/dbc/adbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/adbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/adbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/adbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/adbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/odbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/odbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/odbc.py Outdated Show resolved Hide resolved
py/server/tests/__init__.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/__init__.py Show resolved Hide resolved
@jmao-denver jmao-denver force-pushed the 3131-odbc-integration branch from d23f020 to f836431 Compare January 23, 2023 17:58
@jmao-denver jmao-denver changed the title Relational data ingestion via ODBC Relational data ingestion via ADBC/ODBC Jan 23, 2023
nbauernfeind
nbauernfeind reviewed Jan 24, 2023
View reviewed changes
Copy link
Member

@nbauernfeind nbauernfeind left a comment

Choose a reason for hiding this comment

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

My only comment here is that really the JDBC integration looks exactly like this usage:

cursor.execute("SELECT t_ts, t_exchange, t_price, t_size FROM CRYPTO_TRADES "
                               "LIMIT 1000")

It feels like an extra step pulling this into python's arrow format then translating, with another copy, into a deephaven table.

Wrapping the arrow column sources as accessible directly from DH's engine probably mitigates the concern of yet-another-copy. But, the API looks pretty similar (if not identical) to the java API.

@jmao-denver jmao-denver force-pushed the 3131-odbc-integration branch from f836431 to a0a04b5 Compare January 25, 2023 16:25
@jmao-denver jmao-denver marked this pull request as ready for review January 27, 2023 16:46
chipkent
chipkent requested changes Jan 27, 2023
View reviewed changes
postgres/docker-compose.yml Show resolved Hide resolved
postgres/src/crypto_trades.csv Show resolved Hide resolved
postgres/src/gen_test_data.sql Show resolved Hide resolved
py/server/deephaven/dbc/adbc.py Outdated Show resolved Hide resolved
py/server/deephaven/dbc/__init__.py Outdated Show resolved Hide resolved
chipkent
chipkent previously approved these changes Jan 31, 2023
View reviewed changes
chipkent
chipkent previously approved these changes Jan 31, 2023
View reviewed changes
@jmao-denver jmao-denver force-pushed the 3131-odbc-integration branch from 4043a5f to 3243767 Compare January 31, 2023 21:56
@jmao-denver jmao-denver merged commit 663f093 into deephaven:main Jan 31, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2023
@deephaven-internal
Copy link
Contributor

Labels indicate documentation is required. Issues for documentation have been opened:

How-to: https://github.com/deephaven/deephaven.io/issues/2123
Conceptual: https://github.com/deephaven/deephaven.io/issues/2125
Reference: https://github.com/deephaven/deephaven.io/issues/2124
Blog: https://github.com/deephaven/deephaven.io/issues/2122

@jmao-denver jmao-denver deleted the 3131-odbc-integration branch March 31, 2023 03:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nbauernfeind nbauernfeind nbauernfeind left review comments

@devinrsmith devinrsmith devinrsmith approved these changes

@chipkent chipkent chipkent approved these changes

@jcferretti jcferretti Awaiting requested review from jcferretti jcferretti is a code owner

@rcaudy rcaudy Awaiting requested review from rcaudy rcaudy is a code owner

Assignees

@jmao-denver jmao-denver

Labels
DocumentationNeeded python-server-side ReleaseNotesNeeded Release notes are needed
Projects
None yet
Milestone
Jan 2023
Development

Successfully merging this pull request may close these issues.

ODBC/ADBC integration (import) in Python
6 participants
@jmao-denver @deephaven-internal @nbauernfeind @chipkent @rcaudy @devinrsmith