kevinjqliu commented on PR #3461: URL: https://github.com/apache/iceberg-python/pull/3461#issuecomment-4759290987
I found one propagation gap: `dictionary_columns` reaches `ArrowScan`, but `to_arrow_batch_reader()` wraps the batches with the plain projected schema and casts back to it: `pa.RecordBatchReader.from_batches(target_schema, batches).cast(target_schema)` That erases the dictionary type, so `to_arrow(dictionary_columns=...)` preserves dictionary encoding, but `to_arrow_batch_reader(dictionary_columns=...).read_all()` returns the normal string type. https://github.com/apache/iceberg-python/blob/6da06adfa82eda8d647060632115e75a35634b87/pyiceberg/table/__init__.py#L2242-L2279 Example public-path regression test: ```python def test_to_arrow_batch_reader_preserves_dictionary_columns(catalog: Catalog) -> None: arrow_table = pa.table( { "id": pa.array([1, 2, 3, 4], type=pa.int32()), "label": pa.array(["a", "b", "a", "b"], type=pa.string()), } ) catalog.create_namespace_if_not_exists("default") table = catalog.create_table("default.dict_test", schema=arrow_table.schema) table.append(arrow_table) result = table.scan().to_arrow_batch_reader(dictionary_columns=("label",)).read_all() assert pa.types.is_dictionary(result.schema.field("label").type) assert result.column("label").to_pylist() == ["a", "b", "a", "b"] ``` I didn’t see another user-facing path where the option is accepted and dropped. Minor adjacent cleanup: update the abstract `TableScan.to_arrow` signature so type checkers see the new keyword. -- 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]
