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