Skip to content
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

feat(geo): support geometry type for create table #14615

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

ariesdevil
Copy link
Contributor

@ariesdevil ariesdevil commented Feb 5, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Support geometry type for create table and add two new geometry functions.
If use bendsql, need this patch databendlabs/bendsql#352

part of #14543

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label Feb 5, 2024
@ariesdevil ariesdevil mentioned this pull request Feb 5, 2024
7 tasks
@ariesdevil
Copy link
Contributor Author

ariesdevil commented Feb 5, 2024

we may need install deps for build bounded proj in our ci container. cc @everpcpc

I add the deps for dev_setup.sh in this PR.

@ariesdevil ariesdevil marked this pull request as ready for review February 6, 2024 03:41
@sundy-li sundy-li marked this pull request as draft February 6, 2024 05:14
@sundy-li
Copy link
Member

sundy-li commented Feb 6, 2024

I think it should be supported only if it's stable to avoid incompat problem.

@ariesdevil ariesdevil force-pushed the geometry branch 3 times, most recently from 443e158 to 02c25ae Compare February 7, 2024 10:57
@ariesdevil ariesdevil marked this pull request as ready for review February 7, 2024 11:30
@ariesdevil
Copy link
Contributor Author

ariesdevil commented Feb 7, 2024

parse POINTZ, POINTM and POINTZM for st_geometryfromwkt function will be done after upstream PR merged: georust/wkt#113

As @lnicola kindly point out the Coord in rust geo-types crate just support x and y.

geo-types is a foundational library, almost all libraries under georust depend on it. So if we want to support Point(x,y,z,m) it may require a lot of work to be done.

FYI: Snowflake support these types of point.

@lnicola
Copy link

lnicola commented Feb 7, 2024

the Coord in rust geo-types crate just support x and y

See also georust/geo#797

src/query/sql/src/planner/binder/ddl/table.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/geometry.rs Outdated Show resolved Hide resolved
src/query/functions/src/scalars/geometry.rs Outdated Show resolved Hide resolved
@ariesdevil
Copy link
Contributor Author

@b41sh PTAL again when you have time. Happy Spring Festival:)

Copy link
Member

@b41sh b41sh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please fix the ci tests.

@ariesdevil
Copy link
Contributor Author

ariesdevil commented Feb 19, 2024

image

I have finished the static linking, but the tests still failed (can pass on my machine), and builds for aarch64 and python binding are not successful. @everpcpc need your help.

@everpcpc
Copy link
Member

liblzma and libstdc++ also needs to be static linked.

@ariesdevil ariesdevil force-pushed the geometry branch 4 times, most recently from 340c8bc to 0d23dc7 Compare February 21, 2024 07:12
@ariesdevil
Copy link
Contributor Author

ariesdevil commented Feb 22, 2024

@everpcpc and I have tried various methods diligently this week, but still haven't completely solved the compilation problem of Proj. The current status is:

  1. Proj can be compiled successfully on x86, arm linux and mac m1.

  2. It cannot be cross-compiled from x86 to arm, but Databend relies on this compilation method.

  3. Compilation of Python binding fails; Python binding depends on containers provided by PyO3 for compilation.

Therefore, we temporarily commented out the functions that depend on Proj implementation so we can merge this PR first. We may need more work to complete the compilation of Proj. cc @BohuTANG @sundy-li @kkk25641463 @cdmikechen

@ariesdevil ariesdevil added this pull request to the merge queue Feb 22, 2024
Merged via the queue into databendlabs:main with commit e3aeae6 Feb 22, 2024
71 checks passed
@ariesdevil ariesdevil deleted the geometry branch February 22, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants