zhangfengcdt commented on PR #53:
URL: https://github.com/apache/sedona-db/pull/53#issuecomment-3278089668
> 1. I regret not spotting this issue when I first reviewed the code.
> 2. [suggestion] The `neighbors_geometry` method defined in geo-index does
not have to take an array of `geo::Geometry` as a parameter. We can play all
kinds of tricks in the `distance_metric` function to compute a distance value
given 2 indices of indexed items. This makes the interface more flexible, and
we are free to play all kinds of tricks inside our custom distance metric
function. For instance, decoding the WKB on demand and caching decoded
geometries for reuse.
It's a great suggestion! And, yes, using a custom distance metric with
index-based lookups would be more flexible and could potentially offer even
better performance optimizations. However, I'd like to keep this PR focused on
the immediate performance fix we're addressing. This keeps the current fix
simple and focused while leaving room for future optimization.
The suggested approach would require:
- Changes to the geo-index crate's neighbors_geometry API
- A custom distance metric implementation
- More complex testing across two repositories
- Additional coordination between geo-index and sedona-spatial-join
I think this optimization deserves its own focused PR where we can change
both repos and test them accordingly.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
