rdblue commented on code in PR #6525:
URL: https://github.com/apache/iceberg/pull/6525#discussion_r1067556636
##########
python/pyiceberg/typedef.py:
##########
@@ -126,30 +135,41 @@ def json(self, exclude_none: bool = True, exclude:
Optional[Set[str]] = None, by
class PydanticStruct(IcebergBaseModel):
+ _position_to_field_name: Dict[int, str] = PrivateAttr()
+ _field_name_to_pydantic_field: Dict[str, Field] = PrivateAttr()
+
class Config:
frozen = False
- def __setitem__(self, pos: int, value: Any) -> None:
- positions = list(self.__fields__.values())
- field = positions[pos]
- if value is None:
- if field.default is not None:
- value = field.default
- elif field.default_factory is not None:
- value = field.default_factory()
+ @staticmethod
+ def _get_default_field_value(field: Field) -> Optional[Any]:
+ if field.default is not None:
+ return field.default
+ elif field.default_factory is not None:
+ return field.default_factory()
+ else:
+ return None
+
+ def set_record_schema(self, record_schema: StructType) -> None:
+ self._field_name_to_pydantic_field = {field.name: field for field in
self.__fields__.values()}
+ self._position_to_field_name = {idx: field.name for idx, field in
enumerate(record_schema.fields)}
+ for name, field in self.__fields__.items():
+ setattr(self, name, PydanticStruct._get_default_field_value(field))
- self.__setattr__(field.name, value)
+ def __setitem__(self, pos: int, value: Any) -> None:
+ field_name = self._position_to_field_name[pos]
+ # Check if the field exists
+ if field := self._field_name_to_pydantic_field.get(field_name):
+ self.__setattr__(field.name, value if value is not None else
PydanticStruct._get_default_field_value(field))
Review Comment:
I think that this is incorrect because it assumes that `None` should result
in the default value. The file may explicitly contain a `null` value, in which
case we do not want to default it.
For now, I think it is best if we do not add default values from Pydantic.
We can add that later, but we need to be careful and add them in the readers,
not in the setter.
--
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]
