fix(bigquery): update internal Field.mode type to enum instead of string#13332
fix(bigquery): update internal Field.mode type to enum instead of string#13332logachev wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Field class and its builder to use the Mode enum internally instead of a String representation. While this simplifies getter and setter methods, feedback highlights that changing the type of this serialized field breaks backward serialization compatibility. This could lead to deserialization failures (such as InvalidClassException) in distributed environments like Apache Beam or Spark, so keeping the internal representation as a String is recommended if compatibility must be preserved.
| private final LegacySQLTypeName type; | ||
| private final FieldList subFields; | ||
| private final String mode; | ||
| private final Mode mode; |
There was a problem hiding this comment.
Changing the internal field mode from String to Mode breaks backward serialization compatibility for the Field class, which implements Serializable. If instances of Field (or classes containing it, like Schema) are serialized and shared across different versions of the library (e.g., in distributed processing frameworks like Apache Beam or Spark), deserialization will fail with an InvalidClassException or ClassCastException due to the type mismatch. Since the performance overhead of Mode.valueOf(mode) is negligible, please consider whether breaking serialization compatibility is acceptable for this change. If compatibility must be preserved, it is safer to keep the internal representation as String.
There is constant conversion from string to enum in getter/setter. Updating Field private property to be
Moderather than string