Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Improve the InMemory Catalog Implementation #289
Improve the InMemory Catalog Implementation #289
Changes from 16 commits
066e8c4
b1a99f7
32c449e
3c6e06a
21b1e50
e2541ac
3013bdb
be3eb1c
f80849a
faea973
4437a30
aabcde0
b91e1cf
2834bae
341f5ba
67c028a
96ba8de
c7f9053
8a7b876
10adb1c
a466e5f
03ec82b
58b34ca
810c3c5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
by default, write on disk to
/tmp/warehouse
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.
can pass a warehouse location using properties. warehouse location can be another fs such as s3
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.
Other implementations don't auto-create namespaces, however I think it is fine for the InMemory one.
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.
It looks like we don't write the metadata here, but we write it below at the
_commit
methodThere 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.
yep, the actual writing is done by
_commit_table
below, but the path of the metadata location is determined here.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.
Sorry, but I'm a bit confused here. If I just want to create the table without inserting any data:
I still expect a new
metadata.json
file to be found at the table location without any call to_commit_table
. But that does not seem to be created by the InMemory catalog now. Is there a reason that we choose this behavior?In the previous implementation no file is written. But since we have updated
_commit_table
to write the metadata file, I think it more reasonable to makecreate_table
aligned with other production implementation. WDYT?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.
gotcha, that makes sense!