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: add object store table support for bson format data #2600

Merged
merged 11 commits into from
Feb 22, 2024

Conversation

tychoish
Copy link
Contributor

@tychoish tychoish commented Feb 5, 2024

We maybe could also support lance in a similar sort of way, (which is
where I started when I was looking at this code, but I got this.)

Need to coordinate credentials to get some data uploaded to a gcs
bucet to test this.

Copy link
Contributor

@vrongmeal vrongmeal 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.


I looked into the FileType enum and it being a dependent on FileFormat trait. I think we should implement FileFormat for these formats. We can keep the file_type method as unreachable since that code-path is never used by us.

From slack ^


Ok(provider)
match file_type {
"csv" => Ok(accessor
Copy link
Member

Choose a reason for hiding this comment

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

Apparently the file type is getting stored with with quotes, so matching on "\"csv\"" works...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it's not getting stored as a quote but just printed into a quoted form when it's read which is easy enough clean up.

@tychoish tychoish merged commit 20804d0 into main Feb 22, 2024
25 checks passed
@tychoish tychoish deleted the tycho/bson-object-store-tables branch February 22, 2024 21:05
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.

3 participants