feat: support GEO types

[ddupg]

No description provided.

[github-actions]

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

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.

[ddupg]

The PR is currently blocked by pending releases of datafusion and geodatafusion(geoarrow/geoarrow-rs#1317). These upstream dependencies need to stabilize before we can finalize the geospatial type integration.

[jackye1995]

@ddupg I just merged #4598, some of these can probably be cleaned up and rebased now

[kylebarron]

I'd recommend depending only on geoarrow-array and geoarrow-schema, if those are the only two sub-crates you need. That'll reduce compilation times

[kylebarron]

You can import these from geoarrow_array

[kylebarron]

And these from geoarrow_schema

[kylebarron]

FYI there's a to_field method on PointType, so you don't have to convert to GeoArrowType.

[kylebarron]

FWIW I thought I remembered having a discussion at some point about supporting geospatial indexes in Lance files, though I can't find it anymore.

I thought it would be cool to embed an R-Tree or K-D tree index inside of a lance file and use that for efficient spatial queries. I have the https://github.com/kylebarron/geo-index crate, ported from the original JavaScript which defines a binary format for serializing and deserializing 2D R-Tree/K-D trees (most of the time geospatial support is OK with only 2-dimensional indexes).

If you wanted to push more on geospatial support here, I think adding support for something like geo-index would be super cool. I'd be happy to review PRs but I probably won't have the time to drive that myself.

[jackye1995]

FWIW I thought I remembered having a discussion at some point about supporting geospatial indexes in Lance files, though I can't find it anymore.

I thought it would be cool to embed an R-Tree or K-D tree index inside of a lance file and use that for efficient spatial queries. I have the https://github.com/kylebarron/geo-index crate, ported from the original JavaScript which defines a binary format for serializing and deserializing 2D R-Tree/K-D trees (most of the time geospatial support is OK with only 2-dimensional indexes).

+1

@kylebarron we can probably discuss more in the index PR #4632 or in the discussion at #4482. For all the other files, we also store the index contents in Lance files. It would be great if we can keep it consistent for the geo index. cc @jaystarshot

[kylebarron]

@kylebarron we can probably discuss more in the index PR #4632 or in the discussion at #4482. For all the other files, we also store the index contents in Lance files. It would be great if we can keep it consistent for the geo index. cc @jaystarshot

Agreed; I saw that PR after I commented here!

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[jackye1995]

@ddupg I think we can revive this PR now since all the upgrades are in, is this ready for another look?

[jackye1995]

I think we should at least make this a dedicated feature geo instead of directly in the main cargo. But even with that, we are inflating the dependency further because we have to have all features to do the python binding, which is against what we wanna do in #2224.

This makes me think should we make this a separated lance-geo (or geolance?) package? That also matches the pattern of having geoarrow, geoparquet, etc. as separated projects. What do we think? @westonpace @wjones127 @Xuanwo

[jackye1995]

Or even more simply, we can just make these documentations, so users know how to use it in python & rust, what package to import, how to construct Arrow schema, what DataFusion UDFs to register, etc. We don't really need to have any dedicated code integrations until at some point in the future we are thinking about doing any special encoding for these Geo types, but that seems unlikely for now.

[jackye1995]

Also cc @jaystarshot curious about your opinion here as well.

[ddupg]

Or even more simply, we can just make these documentations, so users know how to use it in python & rust, what package to import, how to construct Arrow schema, what DataFusion UDFs to register, etc. We don't really need to have any dedicated code integrations until at some point in the future we are thinking about doing any special encoding for these Geo types, but that seems unlikely for now.

The current aim is to implement geo index accelerated UDF queries, hence the intention to introduce geo dependencies into Lance. Otherwise, it is indeed possible to rely solely on the documentation.

This makes me think should we make this a separated lance-geo (or geolance?) package?

It is a good idea to make a separated lance-geo package or repo. If so, I'm a bit confused about how Python or Java users would import it.

[jackye1995]

The current aim is to implement geo index accelerated UDF queries, hence the intention to introduce geo dependencies into Lance.

I think we mainly do it through checking the actual function name, and it does not necessary mean you need to import the function?

For example in FTS, we have the FTS query parser as:

fn visit_scalar_function(
        &self,
        column: &str,
        data_type: &DataType,
        func: &ScalarUDF,
        args: &[Expr],
    ) -> Option<IndexedExpression> {
        if args.len() != 2 {
            return None;
        }
        let scalar = maybe_scalar(&args[1], data_type)?;
        if let ScalarValue::Utf8(Some(scalar_str)) = scalar {
            if func.name() == "contains_tokens" {
                let query = TokenQuery::TokensContains(scalar_str);
                return Some(IndexedExpression::index_query(
                    column.to_string(),
                    self.index_name.clone(),
                    Arc::new(query),
                ));
            }
        }
        None
    }

but that does not mean the function really exists, since we are just checking function name here, the function registration can happen elsewhere against the DataFusion context.

I think we can do similar things for geo functions to match names like "st_intersects". What would be the challenge of doing this?

I guess that requires us to align with the positional arguments in the function, but I think that seems reasonable as a way to decouple the 2.

[ddupg]

@ddupg I think we can revive this PR now since all the upgrades are in, is this ready for another look?

There are still a few blocking points at present:

• Waiting the first release version of geodatafusion，geodatafusion Release Timeline datafusion-contrib/geodatafusion#23
• It may be necessary to wait Arrow 57, as the pyo3 conflicts introduced by geoarrow.

[kylebarron]

• Waiting the first release version of geodatafusion，geodatafusion Release Timeline datafusion-contrib/geodatafusion#23

I'm pretty much just trying to flesh out documentation now (e.g. datafusion-contrib/geodatafusion#34). geoarrow 0.6 is published.

• It may be necessary to wait Arrow 57, as the pyo3 conflicts introduced by geoarrow.

You don't seem to be using pyo3-geoarrow, so it doesn't look like your use of geoarrow depends on pyo3 at all

[ddupg]

You don't seem to be using pyo3-geoarrow, so it doesn't look like your use of geoarrow depends on pyo3 at all

I tested with geoarrow 0.6 and geodatafusion 0.1.0-beta and it passed, pyo3 is not blocking.

[ddupg]

The current aim is to implement geo index accelerated UDF queries, hence the intention to introduce geo dependencies into Lance.

I think we mainly do it through checking the actual function name, and it does not necessary mean you need to import the function?

For example in FTS, we have the FTS query parser as:

fn visit_scalar_function(
        &self,
        column: &str,
        data_type: &DataType,
        func: &ScalarUDF,
        args: &[Expr],
    ) -> Option<IndexedExpression> {
        if args.len() != 2 {
            return None;
        }
        let scalar = maybe_scalar(&args[1], data_type)?;
        if let ScalarValue::Utf8(Some(scalar_str)) = scalar {
            if func.name() == "contains_tokens" {
                let query = TokenQuery::TokensContains(scalar_str);
                return Some(IndexedExpression::index_query(
                    column.to_string(),
                    self.index_name.clone(),
                    Arc::new(query),
                ));
            }
        }
        None
    }

but that does not mean the function really exists, since we are just checking function name here, the function registration can happen elsewhere against the DataFusion context.

I think we can do similar things for geo functions to match names like "st_intersects". What would be the challenge of doing this?

I guess that requires us to align with the positional arguments in the function, but I think that seems reasonable as a way to decouple the 2.

Based on offline discussions with @jackye1995 , geospatial index requires parsing geospatial data, which is highly complex. Since geoarrow-rs has already implemented this effectively, we should introduce geoarrow-rs as a dependency into Lance.

geodatafusion does not require direct dependency in Lance, we only need to align with the positional arguments in the function of geodatafusion. Additionally, documentation should be provided explaining how to integrate geodatafusion.

Therefore, I don't think we need a separate pull request for geoarrow. I've already implemented the RTree index #5034 , where geoarrow was introduced.

[jackye1995]

pending CI fix to rebase

[jackye1995]

Sorry merged another PR, could you rebase again?

[ddupg]

Sorry merged another PR, could you rebase again?

Sure thing, I've manually rebased and resolved the conflicts.

[jackye1995]

Thanks for the contribution, this is huge!!! 🚀