You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "milastdbx (via GitHub)" <gi...@apache.org> on 2026/03/31 10:08:59 UTC
[PR] updated geo spec [iceberg]
milastdbx opened a new pull request, #15834:
URL: https://github.com/apache/iceberg/pull/15834
(no comment)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] updated geo spec [iceberg]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3014957872
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
+* `authorithy:identifier` - where `authorithy` represents some well known authorities - examples are `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`.
+* `inlined_projjson` - Inlining whole CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format.
+* `srid:identifier` - [SRID - Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
+* `projjson:table_property_name` - where `table_property_name` is the name of a table property where the projjson string is stored.
Review Comment:
```suggestion
* `authorithy:identifier`- A reference using a well-known authority and its identifier. For example: `OGC:CRS83`.
* `srid:identifier` - A reference using a Spatial Reference Identifier (SRID), where identifier is the numeric SRID value. For example: `SRID:0`.
* `projjson` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification.
* `projjson:table_property_name` - A reference to a CRS definition stored as a table property, where `table_property_name` identifies the property containing the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) string.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3163330286
##########
format/spec.md:
##########
@@ -242,10 +242,13 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<authority>:<code>` - where `<authority>` represents some well known authorities, and `code` is the code used by the authority to identify the CRS. Examples are - `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `EPSG:4326`, `EPSG:3857`, `IGNF:ATI`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions of coordinate reference systems provided by some well known authorities.
+* `srid:<identifier>` - A reference using a [Spatial reference identifier (SRID)](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), where <identifier> is the numeric SRID value. For example: `SRID:0`.
+* `projjson:<table_property_name>` - where <table_property_name> refers to a table property where CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format is stored.
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+CRS identifiers must not contain inlined PROJJSON definitions. If intention is for PROJJSON definition to be part of the table metadata, then it must be stored in a table property and referenced from the CRS field using the `projjson:<table_property_name>` form described above.
Review Comment:
Need to add "Implementations must not parse the contents of the CRS as PROJJSON."
This is strictly invalid and implementations must be explicitly required to reject it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3163317713
##########
format/spec.md:
##########
@@ -242,10 +242,13 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<authority>:<code>` - where `<authority>` represents some well known authorities, and `code` is the code used by the authority to identify the CRS. Examples are - `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `EPSG:4326`, `EPSG:3857`, `IGNF:ATI`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions of coordinate reference systems provided by some well known authorities.
+* `srid:<identifier>` - A reference using a [Spatial reference identifier (SRID)](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), where <identifier> is the numeric SRID value. For example: `SRID:0`.
Review Comment:
I'd roll this into the last bullet so that we don't have a special case for one we don't think will be used.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] updated geo spec [iceberg]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3022587128
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
+* `authorithy:identifier` - where `authorithy` represents some well known authorities - examples are `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`.
Review Comment:
```suggestion
* `authority:identifier` - where `authority` represents some well known authorities - examples are `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3169540813
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
I'm not too bothered by ANY STRING. If we're being overly strict about wording, then it says the string should identify a CRS, not encode a CRS. If you feel like adding a statement to qualify this, you can: "any string that uniquely identifies a CRS (but is not a PROJJSON definition as stated below)".
My guideline for these cases is to go for clarity and I don't think it is unclear. If someone later argues "hey, it said ANY STRING" despite a clear prohibition below, I would have no problem being firm that it was written incorrectly.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3243385778
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<context>:<identifier`: Identifies a CRS by name or other identifier in some well-documented context. Examples: `OGC:CRS84`, `EPSG:4326`, `IGNF:ATI` and `SRID:0`
Review Comment:
The placeholder syntax looks malformed here: ``<identifier>`` is missing the closing `>`.
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<context>:<identifier`: Identifies a CRS by name or other identifier in some well-documented context. Examples: `OGC:CRS84`, `EPSG:4326`, `IGNF:ATI` and `SRID:0`
+* `projjson:<property-name>` - where <property-name> refers to a table property where CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format is stored.
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+CRS value must not contain inlined PROJJSON definitions and implementations must not parse the contents of the CRS as PROJJSON. PROJJSON definition are very verbose, hence inlining them as part of schema would cause significant performance degradation. If intention is for PROJJSON definition to be part of the table metadata, then it must be stored in a table property and referenced from the CRS field using the `projjson:<table_property_name>` form described above.
Review Comment:
A few grammar nits in this paragraph: `PROJJSON definition are` -> `PROJJSON definitions are`, and `If intention is for PROJJSON definition...` would read more naturally as `If the intention is for a PROJJSON definition...`.
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<context>:<identifier`: Identifies a CRS by name or other identifier in some well-documented context. Examples: `OGC:CRS84`, `EPSG:4326`, `IGNF:ATI` and `SRID:0`
Review Comment:
We changed `type` to `context` here. I am not a geo domain expert. I am just wondering if `context` is a widely accepted term in the geo space. is `authority` too restrictive?
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<context>:<identifier`: Identifies a CRS by name or other identifier in some well-documented context. Examples: `OGC:CRS84`, `EPSG:4326`, `IGNF:ATI` and `SRID:0`
+* `projjson:<property-name>` - where <property-name> refers to a table property where CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format is stored.
Review Comment:
This uses `projjson:<property-name>`, but the normative sentence below refers to `projjson:<table_property_name>`. Could we pick one placeholder and use it consistently throughout?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Update wording in Geospatial spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3139010010
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
-1 to having this in Iceberg.
Embedding a raw JSON string in the type was allowed in Parquet because some implementations can't access key-value metadata properties first. But that isn't the case in Iceberg so custom JSON can and should be stored in a table property and referenced here.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3139437189
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
My suggestion for this clarification from our Slack discussion:
> CRS identifiers must not contain PROJJSON definitions. PROJJSON may be stored in a table property and referenced. Implementations must not parse the CRS identifier as PROJJSON.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Update wording in Geospatial spec [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3134530907
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested (but not limited to) formats for CRS are:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
i mean, one is on top, one is on bottom. Its much easier imho if they are next to each other, then one can read the link about projjson and then see the next one. now there's two places, two links.
(popularity may be a bit harder to measure for now, but you may know more than me).
Anyway just my 2c
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#issuecomment-4477537399
the vote passed, so I'll go ahead and merge this. Thanks everyone
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3163324444
##########
format/spec.md:
##########
@@ -242,10 +242,13 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<authority>:<code>` - where `<authority>` represents some well known authorities, and `code` is the code used by the authority to identify the CRS. Examples are - `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `EPSG:4326`, `EPSG:3857`, `IGNF:ATI`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions of coordinate reference systems provided by some well known authorities.
+* `srid:<identifier>` - A reference using a [Spatial reference identifier (SRID)](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), where <identifier> is the numeric SRID value. For example: `SRID:0`.
+* `projjson:<table_property_name>` - where <table_property_name> refers to a table property where CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format is stored.
Review Comment:
Style: The spec uses `kebab-case` for properties, not `snake_case`. This should be `property-name`. (I also recommend using as few words as possible.)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] updated geo spec [iceberg]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3014932093
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
Review Comment:
```suggestion
To maximize interoperability, the following Coordinate Reference System (CRS) formats are recommended (non-exhaustive):
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Update wording in Geospatial spec [iceberg]
Posted by "mkaravel (via GitHub)" <gi...@apache.org>.
mkaravel commented on PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#issuecomment-4298917130
@rdblue @szehon-ho Could you please take a look?
This is the natural follow up of https://github.com/apache/parquet-format/pull/560
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3243445163
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<context>:<identifier`: Identifies a CRS by name or other identifier in some well-documented context. Examples: `OGC:CRS84`, `EPSG:4326`, `IGNF:ATI` and `SRID:0`
Review Comment:
We changed `type` to `context` here. I am not a geo domain expert. I am just wondering if `context` is a widely accepted term in the geo space. both `type` and `context` are very generic. is `authority` too restrictive?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3169522234
##########
format/spec.md:
##########
@@ -242,10 +242,13 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<authority>:<code>` - where `<authority>` represents some well known authorities, and `code` is the code used by the authority to identify the CRS. Examples are - `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `EPSG:4326`, `EPSG:3857`, `IGNF:ATI`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions of coordinate reference systems provided by some well known authorities.
+* `srid:<identifier>` - A reference using a [Spatial reference identifier (SRID)](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), where <identifier> is the numeric SRID value. For example: `SRID:0`.
Review Comment:
Looks like this was not addressed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] updated geo spec [iceberg]
Posted by "mkaravel (via GitHub)" <gi...@apache.org>.
mkaravel commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3018590630
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
+* `authorithy:identifier` - where `authorithy` represents some well known authorities - examples are `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`.
Review Comment:
```suggestion
* `authority:code` - where `authority` represents some well known authorities, and `code` is the code used by the authority to identify the CRS - examples are `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `ESPG:3857`, or `ESRI:37000`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions coordinate reference systems provided by some well known authorities.
```
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
Review Comment:
I would also something about all descriptions being case-insensitive.
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
+* `authorithy:identifier` - where `authorithy` represents some well known authorities - examples are `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`.
+* `inlined_projjson` - Inlining whole CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format.
Review Comment:
It might make sense to add an example for this one as well. How about the one for `OGC:CRS83`?
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
+* `authorithy:identifier` - where `authorithy` represents some well known authorities - examples are `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`.
+* `inlined_projjson` - Inlining whole CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format.
+* `srid:identifier` - [SRID - Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
+* `projjson:table_property_name` - where `table_property_name` is the name of a table property where the projjson string is stored.
Review Comment:
+1 on the description for `srid:identifier` that @uros-db is suggesting.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Update wording in Geospatial spec [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3134459261
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested (but not limited to) formats for CRS are:
Review Comment:
nit: gramatically its better as:
'suggested formats for CRS include, but are not limited to'
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested (but not limited to) formats for CRS are:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
why not put the two 'projjson' together
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] updated geo spec [iceberg]
Posted by "milastdbx (via GitHub)" <gi...@apache.org>.
milastdbx commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3021506459
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
+To maximize interoperability suggested (but not limited to) formats for CRS are:
Review Comment:
I prefer above wording
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "milastdbx (via GitHub)" <gi...@apache.org>.
milastdbx commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3246932662
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<context>:<identifier`: Identifies a CRS by name or other identifier in some well-documented context. Examples: `OGC:CRS84`, `EPSG:4326`, `IGNF:ATI` and `SRID:0`
Review Comment:
thanks for catching these!
Generic word was intentionally used (context) as we wanted to cover broader concepts, such as <authority> and srid:
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "milastdbx (via GitHub)" <gi...@apache.org>.
milastdbx commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3188082984
##########
format/spec.md:
##########
@@ -242,10 +242,13 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<authority>:<code>` - where `<authority>` represents some well known authorities, and `code` is the code used by the authority to identify the CRS. Examples are - `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `EPSG:4326`, `EPSG:3857`, `IGNF:ATI`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions of coordinate reference systems provided by some well known authorities.
+* `projjson:<property-name>` - where <property-name> refers to a table property where CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format is stored.
+* `srid:<identifier>` - A reference using a [Spatial reference identifier (SRID)](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), where <identifier> is the numeric SRID value. For example: `SRID:0`.
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+CRS value must not contain inlined PROJJSON definitions and implementations must not parse the contents of the CRS as PROJJSON. If intention is for PROJJSON definition to be part of the table metadata, then it must be stored in a table property and referenced from the CRS field using the `projjson:<table_property_name>` form described above.
Review Comment:
makes sense to me, updated wording
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] updated geo spec [iceberg]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3014908043
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
Review Comment:
```suggestion
Non-defaullt CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #15834:
URL: https://github.com/apache/iceberg/pull/15834
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "milastdbx (via GitHub)" <gi...@apache.org>.
milastdbx commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3155139750
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
I've updated the spec to reflect this.
However something that bothers me:
- `CRS values are specified by any string that uniquely identifies a coordinate reference system `
Here we are being very explicit that "ANY STRING" is accepted
- `suggested formats for CRS include, but are not limited to`
One more place where we emphasize that these are suggestions and not limitations
Then at the end we do say that PROJJSON is forbidden (contradicts "ANY STRING" above).
While it can be read that this is suggestions + blocklist, which is fine, i still want to make sure we are on same page.
I guess the main question is - do we want for the above suggestions to actually be "limitations" according to spec ?
Wdyt about latest wording ?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Update wording in Geospatial spec [iceberg]
Posted by "mkaravel (via GitHub)" <gi...@apache.org>.
mkaravel commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3134505388
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested (but not limited to) formats for CRS are:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
You mean as the same entry? They are not same if this is what you have in mind.
The plain PROJJSON is expected to be more common than the other one, and the second one is expected to be less common than the authority:code and srid:identifier options. The order in my opinion is good.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Update wording in Geospatial spec [iceberg]
Posted by "milastdbx (via GitHub)" <gi...@apache.org>.
milastdbx commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3136760839
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested (but not limited to) formats for CRS are:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
From parquet discussion, we realized that none of the implementations actually use that, and that its difficult to implement it in practice for parquet readers, hence we left it only for backward compatability purposes.
However, i'd like it to be "discouraged".
Although it is most likely true, that while it makes it hard for parquet writers to implement this, its probably not hard for iceberg writer.
with this in mind - wdyt ?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] updated geo spec [iceberg]
Posted by "uros-db (via GitHub)" <gi...@apache.org>.
uros-db commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3014905743
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-defaullt CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
Review Comment:
```suggestion
Non-default CRS values are specified by any string that uniquely identifies coordinate reference system associated with this type.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Update wording in Geospatial spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3139010010
##########
format/spec.md:
##########
@@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
-
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...`
Review Comment:
-1 to having this in Iceberg.
Embedding a raw JSON string in the type was allowed in Parquet because some implementations can't access key-value metadata properties first. But that isn't the case in Iceberg so custom JSON can and should be stored in a table property and referenced here.
Since this is not clear, I think that we should probably have a note specifically stating that PROJJSON is not allowed here.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3169512254
##########
format/spec.md:
##########
@@ -242,10 +242,13 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<authority>:<code>` - where `<authority>` represents some well known authorities, and `code` is the code used by the authority to identify the CRS. Examples are - `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `EPSG:4326`, `EPSG:3857`, `IGNF:ATI`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions of coordinate reference systems provided by some well known authorities.
+* `projjson:<property-name>` - where <property-name> refers to a table property where CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format is stored.
+* `srid:<identifier>` - A reference using a [Spatial reference identifier (SRID)](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), where <identifier> is the numeric SRID value. For example: `SRID:0`.
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+CRS value must not contain inlined PROJJSON definitions and implementations must not parse the contents of the CRS as PROJJSON. If intention is for PROJJSON definition to be part of the table metadata, then it must be stored in a table property and referenced from the CRS field using the `projjson:<table_property_name>` form described above.
Review Comment:
🚀
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Clarify non-default CRS conventions [iceberg]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #15834:
URL: https://github.com/apache/iceberg/pull/15834#discussion_r3183863714
##########
format/spec.md:
##########
@@ -242,10 +242,13 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a
The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum.
-Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values:
+Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type.
+To maximize interoperability, suggested formats for CRS include, but are not limited to:
+* `<authority>:<code>` - where `<authority>` represents some well known authorities, and `code` is the code used by the authority to identify the CRS. Examples are - `OGC:CRS84`, `OGC:CRS83`, `OGC:CRS27`, `EPSG:4326`, `EPSG:3857`, `IGNF:ATI`. See [https://spatialreference.org/](https://spatialreference.org/) for definitions of coordinate reference systems provided by some well known authorities.
+* `projjson:<property-name>` - where <property-name> refers to a table property where CRS definition in [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) format is stored.
+* `srid:<identifier>` - A reference using a [Spatial reference identifier (SRID)](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), where <identifier> is the numeric SRID value. For example: `SRID:0`.
-* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself.
-* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored.
+CRS value must not contain inlined PROJJSON definitions and implementations must not parse the contents of the CRS as PROJJSON. If intention is for PROJJSON definition to be part of the table metadata, then it must be stored in a table property and referenced from the CRS field using the `projjson:<table_property_name>` form described above.
Review Comment:
It may be a good addition to explain that because PROJJSON is incredibly verbose (~4kb minimum), inlining it here may cause non-trivial performance degredation due to metadata size and make it difficult for implementations to produce reasonable error messages without spatial dependencies. (Or a different justification if this isn't the case!)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org