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.

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: enable passing storage options to Delta table builder via DataFusion's CREATE EXTERNAL TABLE #1043

Merged
merged 3 commits into from
Dec 30, 2022

Conversation

gruuya
Copy link
Contributor

@gruuya gruuya commented Dec 28, 2022

Description

We've recently added Delta table support to Seafowl using delta-rs, which utilizes the new OPTIONS clause in sqlparser/DataFusion. It allows propagating a set of key/values down to the DeltaTableBuilder, which in turn can use those to instantiate a corresponding object store client. This means someone can now define a delta table without relying on env vars as:

CREATE EXTERNAL TABLE my_delta
STORED AS DELTATABLE
OPTIONS ('AWS_ACCESS_KEY_ID' 'secret', 'AWS_SECRET_ACCESS_KEY' 'also_secret', 'AWS_REGION' 'eu-west-3') 
LOCATION 's3://my-bucket/my-delta-table/'

I've also changed the existing datafusion integration tests to use this approach to exercise it.

I'm not sure whether it makes sense to merge this PR upstream, but opening this PR just in case it does.

Related Issue(s)

Didn't find any related issues.

Documentation

@gruuya gruuya changed the title feat: enable passing storage options Delta table builder feat: enable passing storage options to Delta table builder Dec 28, 2022
@gruuya gruuya changed the title feat: enable passing storage options to Delta table builder feat: enable passing storage options to Delta table builder via DataFusion's CREATE EXTERNAL TABLE Dec 28, 2022
houqp
houqp previously approved these changes Dec 30, 2022
@houqp
Copy link
Member

houqp commented Dec 30, 2022

Thanks @gruuya , very cool feature. Looks like the test refactor broke some of the tests, could you fix that?

@gruuya
Copy link
Contributor Author

gruuya commented Dec 30, 2022

Thanks, sure thing!

I realized that I tested the refactored test_datafusion_sql_registration with integration_test feature on, so I didn't catch the build problem. As a resolution I've now moved the new DF test utility function into tests/common/datafusion.

Copy link
Member

@houqp houqp left a comment

Choose a reason for hiding this comment

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

LGTM. @roeap do you want to take another look?

Copy link
Collaborator

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

This looks good to me as well. Thanks for contributing this!

@houqp houqp merged commit 8c67d78 into delta-io:main Dec 30, 2022
@gruuya gruuya deleted the delta-builder-with-storage-options branch March 8, 2023 08:04
chitralverma pushed a commit to chitralverma/delta-rs that referenced this pull request Mar 17, 2023
…usion's CREATE EXTERNAL TABLE (delta-io#1043)

# Description
We've recently added Delta table support to
[Seafowl](https://github.com/splitgraph/seafowl) using delta-rs, which
utilizes the new `OPTIONS` clause in sqlparser/DataFusion. It allows
propagating a set of key/values down to the `DeltaTableBuilder`, which
in turn can use those to instantiate a corresponding object store
client. This means someone can now define a delta table without relying
on env vars as:
```sql
CREATE EXTERNAL TABLE my_delta
STORED AS DELTATABLE
OPTIONS ('AWS_ACCESS_KEY_ID' 'secret', 'AWS_SECRET_ACCESS_KEY' 'also_secret', 'AWS_REGION' 'eu-west-3') 
LOCATION 's3://my-bucket/my-delta-table/'
```

I've also changed the existing datafusion integration tests to use this
approach to exercise it.

I'm not sure whether it makes sense to merge this PR upstream, but
opening this PR just in case it does.

# Related Issue(s)
Didn't find any related issues.

# Documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants