rdblue commented on code in PR #10981:
URL: https://github.com/apache/iceberg/pull/10981#discussion_r1940090275
##########
format/spec.md:
##########
@@ -1480,6 +1497,9 @@ This serialization scheme is for storing single values as
individual binary valu
| **`struct`** | Not supported
|
| **`list`** | Not supported
|
| **`map`** | Not supported
|
+| **`geometry`** | A single point, encoded as a {x, y, optional
z, optional m} concatenation of its 8-byte little-endian IEEE 754 coordinate
values, with the optional coordinates encoded as NaN if unset. |
+| **`geography`** | A single point, encoded as a {x, y, optional
z, optional m} concatenation of its 8-byte little-endian IEEE 754 coordinate
values, with the optional coordinates encoded as NaN if unset. |
Review Comment:
Why encode the Z and M values as NaN if they aren't present? The length of
the value should show what was stored:
* 16 bytes: X and Y
* 24 bytes: X, Y, and Z
* 32 bytes: X, Y, Z, and M
Also, my suggestion to use a binary encoding is to avoid the need for the
WKB type and endianness bytes, which saves 5 bytes per value. Since these are
stored in table metadata, that seems significant to me.
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
