-
Notifications
You must be signed in to change notification settings - Fork 434
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
PARQUET-2471: Add geometry logical type #240
Open
wgtmac
wants to merge
34
commits into
apache:master
Choose a base branch
from
wgtmac:geo
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+311
−0
Open
Changes from 28 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
5c9e110
WIP: Add geometry logical type
wgtmac 5ef28cd
address various comments
wgtmac ecd8cc2
add file level geo stats
wgtmac d81dacb
address feedback:
wgtmac 80f4051
change naming and remove controversial items
wgtmac 0db6d9f
address feedback
wgtmac e817af4
fix typo
wgtmac f78f7bd
use WKB type code
wgtmac 1aaaca8
Update covering and geometry type protocol based on comments (#2)
zhangfengcdt ee5b2df
Add the new suggestion according to the meeting with Snowflake (#3)
jiayuasu 19cc081
change metadata to string type and rewording WKB description
wgtmac 16c5868
add example for crs
wgtmac 56a65de
reword crs
wgtmac f28b282
clarify WKB
wgtmac 5127702
clarify coverings
wgtmac 298ab64
Update the suggestion for bbox stats (#4)
jiayuasu 41c6394
Update src/main/thrift/parquet.thrift
wgtmac d86abe4
Update src/main/thrift/parquet.thrift
wgtmac c7a4f4c
Update src/main/thrift/parquet.thrift
wgtmac f20f685
Update src/main/thrift/parquet.thrift
wgtmac dbf9d54
address feedback about edges and wkb
wgtmac b4296aa
add geoparquet column metadata back
wgtmac 9bcea6e
Update the spec according to the new feedback (#5)
jiayuasu 99f0403
Update src/main/thrift/parquet.thrift
wgtmac dbb78cf
Update src/main/thrift/parquet.thrift
wgtmac 25df0ff
add description to LogicalTypes.md
wgtmac d349727
add explanation for Z & M values
wgtmac 9ea6559
move geo stats to ColumnMetaData
wgtmac 011de45
Update src/main/thrift/parquet.thrift
wgtmac 6425a3c
fix typo
wgtmac 7d8ffa5
Merge branch 'master' into geo
wgtmac 1502458
remove edges and simplify crs
wgtmac 9f53c9e
Add geography type
wgtmac a4f79ca
remove wrong content
wgtmac File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I work with Salesforce Data Cloud team, and evaluating GeoSpatial support in iceberg)
I am new to geospatial world, and wondering what does it mean for edges to be independent of underlying CRS? Can the edges be planar while the CRS is based on elliptic geometry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle, no. First, talking about "planar edges" or "spherical edges" makes no sense and was a confusion of terms in the initial draft of this specification (the group reached an agreement to fix that in recent talks, I hope it will be done before release). An edge can be a straight line, a curve, a geodesic, etc., but cannot be a plane or a sphere (because of wrong number of dimensions).
What the initial draft intended to say with "planar edges" (sic) is "edges computed as if they were in a planar (two-dimensional Cartesian) coordinate system" (the thing that is planar is the coordinate system, not the edges). This is not really correct for geographic CRS, so you are right to said that they are not really independent. However, while it would be more exact to said that lines on a geographic CRS are geodesics, loxodrome, etc., it happens often that software ignore that physical reality and just perform linear interpolations of latitude and longitude values. The line on the ellipsoid surface obtained that way has no interesting properties, it is just easy to compute. We do not recommend doing that, but the use of "planar" word in this context was an acknowledgement that it happens in practice and an attempt to describe that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the response. I do not understand what this parameter is used for in parquet. If it is the engine's property to treat the edges, how is this value helping? The engine capable of interpreting edges as geodesics should do so if the CRS reference indicates that the underlying geometry column belongs to an ellipsoid datum. Is this edge property forcing the engine to treat the values in a planar coordinate system?
In other words, is there something intrinsic to the data stored in the parquet file itself where edge parameter makes a difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@redblackcoder
The Geo and Iceberg community are discussing the best way to describe this field. It is very likely that we will want to rename
edges
property to something else because this is not what we want to describe initially. We will post updates in a few days.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the most common case, SRID 4326. It is Geographic coordinate system (GEOGCS) rather than Projected one.
https://www.esri.com/arcgis-blog/products/arcgis-pro/mapping/gcs_vs_pcs/
So the linestring from A to B should follow the geodesic line. But most systems treat 4326 as planar map. E.g. with Geometry type in PostGIS or MS SQL Server, they treat it as projected coordinate system, and the linestrings follow straight lines on flat surface. If you use latest MySQL or Geography type in PostGIS or MS SQL Server, the linestrings in 4326 follow geodesic lines on sphere. So there is ambiguity what exactly a linestring or polygon in 4326 describes. Is
'point(30 21)
insidepolygon((10 10, 50 10, 50 20, 10 20, 10 10))
?With geometry, in PostGIS, returns false:
Same thing with geography (4326 is presumed), returns true:
Unfortunately, there is no accepted way to describe the difference between geometry and geography in WKB format. You can encounter SRID=4326 with both interpretations. The
edge
attribute allows describing the difference between geometry and geography, and tells user how to interpret the data in a way consistent with the system that produced it.