rdblue commented on code in PR #12346:
URL: https://github.com/apache/iceberg/pull/12346#discussion_r2008308735


##########
api/src/main/java/org/apache/iceberg/types/Types.java:
##########
@@ -58,9 +58,16 @@ private Types() {}
           .put(BinaryType.get().toString(), BinaryType.get())
           .put(UnknownType.get().toString(), UnknownType.get())
           .put(VariantType.get().toString(), VariantType.get())
+          .put(GeometryType.crs84().toString(), GeometryType.crs84())
+          .put(GeographyType.crs84().toString(), GeographyType.crs84())
           .buildOrThrow();
 
   private static final Pattern FIXED = 
Pattern.compile("fixed\\[\\s*(\\d+)\\s*\\]");
+  private static final Pattern GEOMETRY_PARAMETERS =
+      Pattern.compile("geometry\\s*(?:\\(\\s*([^,]+?)\\s*\\))?", 
Pattern.CASE_INSENSITIVE);

Review Comment:
   I typically like to match on any character other than the next delimiter, 
which in this case is `)`. Rather than matching any non-comma character with 
`([^,]+?)`, using `([^)]+?)` instead will cover more cases.
   
   We don't want a string to fail matching the regex but that's what would 
happen in cases like `geometry(srid:123,456)`. Instead of failing to match, I 
think it will be better to parse correctly and then state that the CRS is 
unknown.



-- 
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

Reply via email to