geruh commented on PR #3293: URL: https://github.com/apache/iceberg-python/pull/3293#issuecomment-4348885514
> I'm torn on inspect. There's value in a human understanding that the data on disk stating that the file is of a particular type. I didn't change it, but I'm happy to be overridden on that one. I did a brief pass earlier so I wasn't able to give much context. Inspect methods are calling [`from_bytes`](https://github.com/apache/iceberg-python/blob/main/pyiceberg/table/inspect.py#L183) for the bounds, and when type promotions come into play by that logic this will fail right. Because just like what you're laying down here, `lower/upper_bound` is the raw bytes from the manifest, and `field.field_type` comes from `self.tbl.metadata.schema()` which is the promoted type. This can be proved by adding an integration test like: ```python schema = Schema(NestedField(1, "val", IntegerType(), required=False)) tbl = catalog.create_table("glue.dru", schema) tbl.append(pa.table({"val": pa.array([10, 20, 30], type=pa.int32())})) with tbl.update_schema() as upd: upd.update_column("val", field_type=LongType()) tbl.inspect.entries() ``` running this gives me: ``` struct.error: unpack requires a buffer of 8 bytes ``` Arguably, this makes the `conversions.py` dispatch approach a little more appealing to me since we can fix it for everything in one place. but, would need to see or try it out. wdyt? -- 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]
