feat: support GEO RTree index #5034
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ddupg
force-pushed
the
feat/geo-type
branch
2 times, most recently
from
101e660 to
5352673
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Kontinuation
left a comment
Kontinuation
left a comment
There was a problem hiding this comment.
I'm very interested in your work. After a preliminary review of the code, I have a few suggestions/comments
ddupg
force-pushed
the
feat/geo-type
branch
3 times, most recently
from
135714d to
75c7b08
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
ddupg
force-pushed
the
feat/geo-type
branch
2 times, most recently
from
c00e492 to
e1ffeb1
Compare
|
The python example is currently blocked at #5144 |
ddupg
force-pushed
the
feat/geo-type
branch
5 times, most recently
from
fbdd59f to
49cfef2
Compare
ddupg
force-pushed
the
feat/geo-type
branch
3 times, most recently
from
5b53354 to
0b29e09
Compare
There was a problem hiding this comment.
And one other thing is that we should raise a separated PR + vote to add RTree to the table index spec, treat it as a spec change. This follows the new community governance rule, and also allows a pre-review of the general design. We did not do that for zone map and it caused quite a few issues, so I think let's follow the new process here.
There was a problem hiding this comment.
Thanks for the suggestion! I’ve raised a PR #5360 outlining the RTree index spec.
ddupg
force-pushed
the
feat/geo-type
branch
11 times, most recently
from
d4473b1 to
4484700
Compare
ddupg
force-pushed
the
feat/geo-type
branch
2 times, most recently
from
b67322d to
58602a7
Compare
This PR proposes adding the R-Tree index specification to the Lance table format. For implementation details please see #5034 Feel free to leave comments or share feedback
ddupg
force-pushed
the
feat/geo-type
branch
3 times, most recently
from
4ec386f to
65c6e78
Compare
This PR proposes adding the R-Tree index specification to the Lance table format. For implementation details please see lance-format#5034 Feel free to leave comments or share feedback
ddupg
force-pushed
the
feat/geo-type
branch
3 times, most recently
from
e9e4628 to
f3736eb
Compare
Change-Id: Ic057263d3ff4e9dc7d5874e0b92687b551cfb836
|
Could @jackye1995 please review this again when you have time? |
jackye1995
left a comment
jackye1995
left a comment
There was a problem hiding this comment.
looks good to me! I think all the old comments are addressed, and I am good with the current implementation. Thanks for pushing this through!
4 participants
No description provided.