rdblue commented on code in PR #11831:
URL: https://github.com/apache/iceberg/pull/11831#discussion_r1942081865
##########
core/src/main/java/org/apache/iceberg/SchemaParser.java:
##########
@@ -42,6 +42,7 @@ private SchemaParser() {}
private static final String STRUCT = "struct";
private static final String LIST = "list";
private static final String MAP = "map";
+ private static final String VARIANT = "variant";
Review Comment:
Ah, thanks for pointing out that `fromPrimitiveString` returns
`PrimitiveType`. That won't work then.
We do want places like `PrimitiveHolder` to work with Variant, since it is
an unparameterized type. We should be using it to serialize and maintain
`VariantType` as a singleton. I'd probably update it to use `fromString` and
rename it if you think it is confusing (It could be
`UnparameterizedTypeHolder`, but it isn't a big deal because it is
package-private).
For the new method, I think we need to find a good name. `fromString` is
possible, but it doesn't capture that `list`, `struct`, and `map` types aren't
supported. What about `fromTypeName`? Then it is maybe a little more clear that
you can't construct a `struct` type from just a name?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
