-
Notifications
You must be signed in to change notification settings - Fork 477
LTREE data type
#20810
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
LTREE data type
#20810
Conversation
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Looks great! I left two minor suggestions.
Also, as I was reviewing these docs and trying out LTREE features, I found three bugs!
I've marked them as known limitations for v25.4.0.
src/current/v25.4/ltree.md
Outdated
|
|
||
| ## Size | ||
|
|
||
| The size of an `LTREE` value is variable and equals the total number of characters in all labels plus the dot separators. The maximum label length is 1,000 characters, and the maximum number of labels in a path is 65,535. |
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.
That makes the max LTREE ~65MB. For STRING values we recommend keeping them under 64 kB:
https://www.cockroachlabs.com/docs/stable/string#size
The size of a STRING value is variable, but it's recommended to keep values under 64 kilobytes to ensure performance. Above that threshold, write amplification and other considerations may cause significant performance degradation.
Since an LTREE is stored like a STRING, I think we should make a similar recommendation 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.
We actually currently have conflicting recommendations for BYTES and STRING even though they should be the same. This was mentioned just last week and Rich filed https://cockroachlabs.atlassian.net/browse/DOC-15179 about that. (I actually don't know what the recommendation should be nowadays, we'll need to figure that out.)
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.
Noted -- given Marcus's feedback I'll use the STRING guidance here for now, and if necessary it can be corrected after @rmloveland progresses on that issue.
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.
Nice work!
src/current/v25.4/ltree.md
Outdated
|
|
||
| ## Size | ||
|
|
||
| The size of an `LTREE` value is variable and equals the total number of characters in all labels plus the dot separators. The maximum label length is 1,000 characters, and the maximum number of labels in a path is 65,535. |
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.
We actually currently have conflicting recommendations for BYTES and STRING even though they should be the same. This was mentioned just last week and Rich filed https://cockroachlabs.atlassian.net/browse/DOC-15179 about that. (I actually don't know what the recommendation should be nowadays, we'll need to figure that out.)
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.
Just one small comment, otherwise LGTM!
src/current/v25.4/ltree.md
Outdated
|
|
||
| ## Size | ||
|
|
||
| The size of an `LTREE` value is variable and equals the total number of characters in all labels plus the dot separators. Cockroach Labs recommends keeping values below 64 kilobytes. Above that threshold, [write amplification]({% link {{ page.version.version }}/architecture/storage-layer.md %}#write-amplification) and other considerations may cause significant performance degradation. |
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 here I would say "plus the total number of dot separators" just for clarity/consistency
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.
Agree. Fixed!
https://cockroachlabs.atlassian.net/browse/DOC-15227
LTREEdata typePreview: https://deploy-preview-20810--cockroachdb-docs.netlify.app/docs/v25.4/ltree.html