Spec: Allow spaces in UDF type strings - #18057
Conversation
|
|
||
| Type strings are used exactly as Iceberg serializes them. Parameterized types may contain spaces, either separating | ||
| parameters (`decimal(9, 2)`, `geography(OGC:CRS84, spherical)`) or within a parameter value (`geometry(srid: 3857)`). | ||
| Readers must not add or remove spaces in a type string. |
There was a problem hiding this comment.
This is phrased a bit too broadly. The intent is that whitespace inside a type string is semantically significant and must not be stripped or normalized during comparison (geometry(srid: 3857) ≠ geometry(srid:3857), CRS comparison is case-insensitive but not space-insensitive per GeometryType.equals()). However, the rule as written could be read as prohibiting write-side canonicalization, e.g. the normalization path in the upcoming #15994 (Types.fromTypeName(s).toString()) deliberately adds a space when given old-form decimal(9,2) → decimal(9, 2). Please consider rephrasing to scope it to comparison semantics: "Implementations must compare type strings exactly as written; whitespace inside a type string is semantically significant and must not be stripped or normalized for comparison purposes."
There was a problem hiding this comment.
Agree that write-side canonicalization should remain allowed. I think “compare exactly as written” is still too broad because Appendix C requires readers to accept optional whitespace around parameters and separators, so decimal(9,2) and decimal(9, 2) should resolve to the same type.
Could we instead say:
Writers must use Iceberg’s canonical serialized form. Readers should accept optional whitespace around parameters and separators. Implementations must compare parsed or canonicalized types, not raw type strings. Canonicalization may normalize syntactic whitespace but must preserve whitespace within parameter values.
This keeps Types.fromTypeName(s).toString() valid while preserving meaningful CRS whitespace such as in srid: 3857.
|
Shall we also loop in @szehon-ho here, given the context in geospatial |
| * Map: `map<key-type,value-type>` (e.g., `map<string,int>`) | ||
| * Struct: `struct<name1:type1,name2:type2,...>` with field names and types (e.g., `struct<id:int,name:string>`) | ||
|
|
||
| A type string is embedded unchanged, so a parameterized type may still contribute spaces of its own (see [Types](#types)). |
There was a problem hiding this comment.
Please derive definition-id from normalized type identity rather than embedding the serialized string unchanged. Iceberg treats geometry/geography CRS values case-insensitively, but serialization preserves the supplied casing, so equal types such as geometry(EPSG:4326) and geometry(epsg:4326) can otherwise produce different IDs. That conflicts with the requirement that only one definition exist per signature. The spec should define the normalization used for the ID, or explicitly define UDF type identity differently from Iceberg type equality.
|
Yea , thanks @uros-b i'll also take a look at this, actually i think it came from my comment on : #15994 (comment) there's a discrepancy between Iceberg table spec and UDF spec in serialization of the geo type |
The UDF spec requires that type strings "contain no spaces or quote characters", but Iceberg
serializes parameterized type strings with a space after each parameter separator —
decimal(9, 2),geography(OGC:CRS84, spherical). No implementation that uses Iceberg's owntype serialization can satisfy that rule, and the spec's own
decimal(9,2)example does notconform to it either.
The spaces can't be normalized away, because a space inside a parameter value is significant:
geometry(srid: 3857)andgeometry(srid:3857)are different types.This PR:
compared exactly as Iceberg serializes them.
decimal(9,2)example todecimal(9, 2).definition-id, but scopes it to the separators that thedefinition-id format itself adds. An embedded type string keeps its own spaces.
Prerequisite for #15994.