feat: support GEO RTree index #5034

ddupg · 2025-10-22T12:22:31Z

No description provided.

chatgpt-codex-connector

💡 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".

rust/lance-index/src/scalar.rs

codecov-commenter · 2025-10-23T13:08:49Z

Codecov Report

❌ Patch coverage is 77.64706% with 285 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance-index/src/scalar/rtree.rs	83.57%	76 Missing and 39 partials ⚠️
rust/lance-geo/src/bbox.rs	65.32%	65 Missing and 4 partials ⚠️
rust/lance-index/src/scalar/expression.rs	34.88%	55 Missing and 1 partial ⚠️
.../lance-index/src/scalar/rtree/sort/hilbert_sort.rs	82.38%	24 Missing and 7 partials ⚠️
rust/lance-index/src/scalar.rs	74.00%	13 Missing ⚠️
rust/lance-index/src/lib.rs	50.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Kontinuation

I'm very interested in your work. After a preliminary review of the code, I have a few suggestions/comments

rust/lance-index/src/scalar/rtree.rs

chatgpt-codex-connector

💡 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".

rust/lance-index/src/scalar.rs

ddupg · 2025-11-05T08:43:50Z

The python example is currently blocked at #5144

jackye1995 · 2025-11-26T08:01:05Z

Cargo.toml

    "rust/lance-encoding",
    "rust/lance-file",
    "rust/lance-index",
+    "rust/lance-geo",


this is a pretty big change, I would suggest we have a PR to get the lance-geo module in first, with the tests for geo type and functions.

OK，I separate to #4678, please help review it.

jackye1995 · 2025-11-26T08:01:30Z

Cargo.toml

 ] }
 crossbeam-queue = "0.3"
-datafusion = { version = "50.0.0", default-features = false, features = [
+datafusion = { version = "50.3.0", default-features = false, features = [


I think let's bump datafusion in a separate PR?

This upgrade may be unnecessary, I just wanted to keep it consistent with the lock file. I noticed that bump datafusion 51 is in progress, so I've rolled back this bump.

jackye1995 · 2025-11-26T08:16:30Z

rust/lance-geo/src/bbox.rs

+
+/// Convert a length-1 GeoArrowArray to a geo::Geometry scalar.
+pub fn bounding_box_single_scalar(arr: &dyn GeoArrowArray) -> ArrowResult<BoundingBox> {
+    assert_eq!(arr.len(), 1);


prefer error not panic

The method is unnecessary, removed it.

jackye1995 · 2025-11-26T08:18:50Z

rust/lance-index/src/scalar/rtree/sort/hilbert_sort.rs

+        let hilbert_max = ((1 << 16) - 1) as f64;
+        let len = rect_array.len();
+        let width = self.bbox.maxx() - self.bbox.minx();
+        let height = self.bbox.maxy() - self.bbox.miny();


what if the points are in the same x or y axis? seems like we need a minimum for width and height that is not zero, since I remember hilbert curve does not divide by 0 (if it does then never mind)

Good point. In fact, division-by-zero will not occur. If all x are 0, in the subsequent expression, (bbox.minx() + bbox.maxx()) / 2. - self.bbox.minx() will yield 0, resulting in the entire expression being 0.

let x = (hilbert_max * ((bbox.minx() + bbox.maxx()) / 2. - self.bbox.minx()) / width) .floor() as u32;

To be safe, I've also updated the logic to guard against division-by-zero by setting the width/height to 1.0 if max equals min. This approach aligns the one used in flatbush (lines 158-171) which effectively ensures the normalization accuracy.

jackye1995 · 2025-11-26T18:16:54Z

rust/lance-index/src/scalar/rtree.rs

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.

Thanks for the suggestion! I’ve raised a PR #5360 outlining the RTree index spec.

Change-Id: Ic057263d3ff4e9dc7d5874e0b92687b551cfb836

github-actions bot added enhancement New feature or request python java labels Oct 22, 2025

ddupg mentioned this pull request Oct 22, 2025

Tracking issue for Geo type and index #5035

Open

3 tasks

chatgpt-codex-connector bot reviewed Oct 22, 2025

View reviewed changes

rust/lance-index/src/scalar.rs Outdated Show resolved Hide resolved

ddupg force-pushed the feat/geo-type branch from a11b100 to 1bd1f08 Compare October 22, 2025 12:29

ddupg mentioned this pull request Oct 22, 2025

feat: support GEO types #4678

Merged

ddupg marked this pull request as draft October 22, 2025 12:52

ddupg mentioned this pull request Oct 22, 2025

feat: support Point BKD Tree Geo Index and st_within interception #4632

Open

ddupg force-pushed the feat/geo-type branch 2 times, most recently from 101e660 to 5352673 Compare October 23, 2025 10:11

ddupg force-pushed the feat/geo-type branch from c0b45a8 to 6d48b19 Compare October 24, 2025 02:29

Kontinuation reviewed Oct 28, 2025

View reviewed changes

rust/lance-index/src/scalar/rtree.rs Outdated Show resolved Hide resolved

rust/lance-index/src/scalar/rtree.rs Show resolved Hide resolved

ddupg force-pushed the feat/geo-type branch 3 times, most recently from 135714d to 75c7b08 Compare October 28, 2025 09:37

ddupg marked this pull request as ready for review October 28, 2025 10:43

chatgpt-codex-connector bot reviewed Oct 28, 2025

View reviewed changes

rust/lance-index/src/scalar.rs Outdated Show resolved Hide resolved

ddupg force-pushed the feat/geo-type branch 2 times, most recently from c00e492 to e1ffeb1 Compare November 3, 2025 09:50

ddupg force-pushed the feat/geo-type branch 5 times, most recently from fbdd59f to 49cfef2 Compare November 11, 2025 02:42

ddupg force-pushed the feat/geo-type branch 3 times, most recently from 5b53354 to 0b29e09 Compare November 19, 2025 14:07

ddupg force-pushed the feat/geo-type branch 7 times, most recently from d15373f to 3966395 Compare November 26, 2025 02:03

jackye1995 reviewed Nov 26, 2025

View reviewed changes

ddupg force-pushed the feat/geo-type branch 2 times, most recently from 918adb1 to 9d0ab4d Compare November 26, 2025 15:05

jackye1995 reviewed Nov 26, 2025

View reviewed changes

ddupg force-pushed the feat/geo-type branch from 9d0ab4d to 8f7de32 Compare November 27, 2025 06:15

ddupg mentioned this pull request Nov 27, 2025

feat: add RTree index spec in table format #5360

Open

feat: support GEO RTree index

ff5fd61

Change-Id: Ic057263d3ff4e9dc7d5874e0b92687b551cfb836

ddupg force-pushed the feat/geo-type branch from 8f7de32 to ff5fd61 Compare November 27, 2025 12:04

Merge branch 'main' into feat/geo-type

46fac04

feat: support GEO RTree index #5034

Are you sure you want to change the base?

feat: support GEO RTree index #5034

Conversation

ddupg commented Oct 22, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

codecov-commenter commented Oct 23, 2025 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Kontinuation left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

ddupg commented Nov 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Oct 23, 2025 •

edited by codecov bot

Loading