-
Notifications
You must be signed in to change notification settings - Fork 592
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
feat: implement read_delta for datafusion #6411
feat: implement read_delta for datafusion #6411
Conversation
ibis/backends/datafusion/__init__.py
Outdated
# Our other backends support overwriting views / tables when reregistering | ||
# TODO: datafusion doesn't natively support Delta Lake tables at this point, | ||
# so we need to add this later? | ||
# self._context.deregister_table(table_name) |
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.
or should this line be uncommented still?
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.
I don't think whether support is native or not matters here, so this line should be uncommented.
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.
that results in an error:
> self._context.register_delta(table_name, source_table, **kwargs)
E AttributeError: 'datafusion.SessionContext' object has no attribute 'register_delta'
ibis/backends/datafusion/__init__.py:275: AttributeError
ef4a53d
to
7faaf3c
Compare
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.
LGTM! Thanks!
closes #6384
one question is noted in the TODO -- my understanding is for csv and parquet we're using a native datafusion function for de-registering the files from the database, but we can't do that with delta yet?
datafusion has an issue open for native support: apache/datafusion#2025
also cleaned up a docstring issue I noticed in duckdb