-
Notifications
You must be signed in to change notification settings - Fork 24
docs: add partitioin spec #279
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
base: main
Are you sure you want to change the base?
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
ACTION NEEDED The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. For details on the error please inspect the "PR Title Check" action. |
| @@ -0,0 +1,161 @@ | |||
| ## Lance Partition Spec (Experimental) | |||
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.
One thought, can we make this a part of the DirectoryNamespace catalog spec, because it is basically a directory namespace + all tables must have the same schema, and the namespace must be leveled.
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.
Thanks your suggestion! I'm not sure if I understand correctly. Do you mean "the Partition Spec should be part of the Directory Spec"?
I’m thinking if we could move the "Partition Spec" as an individual file to the docs/src directory? This is because I thought the Partition Spec should be applicable to the REST Spec, Directory Spec, and External Spec simultaneously, thus we don’t need to restrict the use of Partition Namespace to the Directory Spec alone.
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.
In my mind any container for the data must be storage only, which is a principle that Lance holds for important reasons like portability. So a partitioned namespace itself should have a fixed storage-only definition, and directory namespace can be the basis. It can be a separated file, but it should basically be something like "a partitioned namespace is a special directory namespace (v2+) that adds the following constraints...".
And then any namespace implementation (Hive, or your product catalog, or REST) can let its namespace contain a partitioned namespace, that is a catalog integration and fits well into the whole namespace framework.
What do you think about it?
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.
Thanks your explanation! It makes sense to me. Let me fix it.
docs/src/rest/partition-spec.md
Outdated
| This specification follows the "convention over configuration" principle, defining a partitioned table as a **metadata specification**. It is the responsibility of compatible clients or computation engines to interpret and enforce it. | ||
|
|
||
| ### 2. Core Concepts | ||
| #### 2.1. Partitioned Table |
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 think calling this a partitioned table would be quite confusing to many people because the actual interface is namespace not table. What about "Partitioned Namespace"?
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.
👌
9262dcb to
5b893f0
Compare
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
5b893f0 to
ce8bb5c
Compare
ce8bb5c to
bd6fb89
Compare
|
Hi @jackye1995 , I've updated this pr, could you help review when you have time, thanks very much~ |
This pr tries to add partition as an experimental spec. Discussion could be fount at: #272
This PR remains a draft, and we still need to hold a vote to decide whether to introduce the partition spec.