Fokko commented on code in PR #848:
URL: https://github.com/apache/iceberg-python/pull/848#discussion_r1667417924
##########
pyiceberg/catalog/__init__.py:
##########
@@ -675,8 +675,11 @@ def _convert_schema_if_needed(schema: Union[Schema,
"pa.Schema"]) -> Schema:
from pyiceberg.io.pyarrow import _ConvertToIcebergWithoutIDs,
visit_pyarrow
+ downcast_ns_timestamp_to_us =
Config().get_bool("downcast-ns-timestamp-to-us-on-write") or False
Review Comment:
Nit: we can move `"downcast-ns-timestamp-to-us-on-write"` into a constant,
and reuse it in `pyarrow.py`
##########
pyiceberg/io/pyarrow.py:
##########
@@ -918,11 +919,24 @@ def primitive(self, primitive: pa.DataType) ->
PrimitiveType:
return TimeType()
elif pa.types.is_timestamp(primitive):
primitive = cast(pa.TimestampType, primitive)
- if primitive.unit == "us":
- if primitive.tz == "UTC" or primitive.tz == "+00:00":
- return TimestamptzType()
- elif primitive.tz is None:
- return TimestampType()
+ if primitive.unit in ("s", "ms", "us"):
+ # Supported types, will be upcast automatically to 'us'
+ pass
+ elif primitive.unit == "ns":
+ if Config().get_bool("downcast-ns-timestamp-on-write"):
Review Comment:
> I also failed to reproduce this issue in my environment (possibly because
it has pandas installed) so I'm reverting this change for now.
Ah, of course. One of the few upsides of having a new Macbook.
--
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
