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

#240 support s3 direct with ColumnQObjectStoreProvider #241

Merged
merged 9 commits into from
Jan 17, 2023

Conversation

jychen7
Copy link
Collaborator

@jychen7 jychen7 commented Jan 14, 2023

What

part 1 of #240, also close #227

Why

before this PR, there is error about

Error: DataFusion error: Internal error: No suitable object store found for s3://***

Because it creates a new SessionContext and such context have no registry or provider for s3

let ctx = SessionContext::new();
options.infer_schema(&ctx.state(), &table_url).await?

How

This PR adds ObjectStoreProvider to the "global" SessionContext and pass it down to columnq/src/table/parquet.rs

For Reviewer

The PR is ready to review with

  • integration test of s3 direct
  • unit test of object store provider

columnq/src/columnq.rs Outdated Show resolved Hide resolved
use datafusion::datasource::object_store::ObjectStoreProvider;
use url::Url;

use super::ColumnQObjectStoreProvider;
Copy link
Collaborator Author

@jychen7 jychen7 Jan 17, 2023

Choose a reason for hiding this comment

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

@houqp PR is ready for review, including both unit tests and integration tests

Thanks

@jychen7 jychen7 requested a review from houqp January 17, 2023 02:11
@jychen7 jychen7 marked this pull request as ready for review January 17, 2023 02:11
@houqp houqp merged commit 1e3644e into roapi:main Jan 17, 2023
@houqp
Copy link
Member

houqp commented Jan 17, 2023

Exciting stuff, thanks @jychen7 !

@houqp
Copy link
Member

houqp commented Jan 17, 2023

I think it's about time to create a new release since there are a lot of improvements added since the last one. @jychen7 do you have any major feature you plan to add before the release.

@jychen7 jychen7 deleted the 227-s3-direct branch January 17, 2023 14:03
@jychen7
Copy link
Collaborator Author

jychen7 commented Jan 17, 2023

@houqp no, I think it is good to create a release.

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

Successfully merging this pull request may close these issues.

Parquet s3 urls not working when use_memory_table is set to false
2 participants