avison9 commented on code in PR #3595:
URL: https://github.com/apache/iceberg-python/pull/3595#discussion_r3531970091


##########
pyiceberg/table/__init__.py:
##########
@@ -2196,6 +2196,20 @@ def _to_arrow_batch_reader_via_file_scan_tasks(
     from pyiceberg.io.pyarrow import ArrowScan, schema_to_pyarrow
 
     target_schema = schema_to_pyarrow(projected_schema)
+    if dictionary_columns:
+        """schema_to_pyarrow returns plain types. ArrowScan yields
+           dictionary-encoded batches for the requested columns, so rebuild
+           target_schema with dictionary types for those fields. Without this,
+           .cast(target_schema) would silently convert dictionary arrays back
+           to their plain value type and erase the encoding."""
+        dict_col_set = set(dictionary_columns)
+        target_schema = pa.schema(
+            [
+                field.with_type(pa.dictionary(pa.int32(), field.type)) if 
field.name in dict_col_set else field

Review Comment:
   @Fokko, presently for every code path that actually reaches this line, it 
will always be `int32`. But that's because of an internal choice in Arrow's C++ 
Parquet reader, not something PyArrow documents or guarantees anyway.
   If a future PyArrow version ever changed that reader's behavior (e.g., to 
shrink to int8/int16 for low-cardinality columns, matching 
`dictionary_encode()`'s behavior), the hardcoded `int32 `would silently be 
wrong again, in
   exactly the same class of bug this PR is fixing.
   The best solution will be to derive the actual dictionary field from the 
first real batch `ArrowScan` produces, peek one batch, read its actual index 
type off `first_batch.schema.field(name)`, and prepend that batch back onto the 
stream before building target_schema.
   I have added this in a new commit, if you can please take a look



-- 
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]

Reply via email to