Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes #1269820
Fill out the following pre-review checklist:
[ ] I am adding a new automated test(s) to verify correctness of my new code
[ ] I am adding new logging messages
[ ] I am adding a new telemetry message
[ ] I am modifying authorization mechanisms
[ ] I am adding new credentials
[ ] I am modifying OCSP code
[ ] I am adding a new dependency
Please describe how your code solves the related issue.
The main purpose of this change is to add tests that show that structured types work with iceberg tables and conversion to pandas. While implementing the tests I found a number of gaps in that functionality which I've either patched to add support, or opened relevant tickets to do so in the future.
Review Notes:
CArrowTableIterator.cpp
I pulled most of the logic out from reconstructRecordBatches_nanoarrow into a new function convertIfNeeded. This allows me to call it recursively when parsing nested types. I've added logic to support ARRAY, MAP, and OBJECT to the switch in this function.
Most of the converters in this file have ben slightly modified to no longer rely on indexes, but to instead use pointers. This allows calling them from an arbirary nesting level when parsing structured types.
There's a couple spots in the converters where I simplified the code slightly to make it more readable.
result_batch.py
By default arrow would cast maps to something like [('k1', 1), ('k2', 2)] which is not the most intuitive thing to work with. pyspark would return {'k1': 1, 'k2': 2} which is a better form in my opinion. The parameter added in this file will default to the pyspark style return. This should not be a BCR because this map type is not publicly available yet.
test_arrow_result.py
I've refactored the tests so that they parameterize on the matrix of [datatype, iceberg, pandas]. Each datatype is tested against FDN and iceberg tables and collected to either python objects or pandas dataframes.
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes #1269820
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
The main purpose of this change is to add tests that show that structured types work with iceberg tables and conversion to pandas. While implementing the tests I found a number of gaps in that functionality which I've either patched to add support, or opened relevant tickets to do so in the future.
Review Notes:
reconstructRecordBatches_nanoarrow
into a new functionconvertIfNeeded
. This allows me to call it recursively when parsing nested types. I've added logic to support ARRAY, MAP, and OBJECT to the switch in this function.[('k1', 1), ('k2', 2)]
which is not the most intuitive thing to work with. pyspark would return{'k1': 1, 'k2': 2}
which is a better form in my opinion. The parameter added in this file will default to the pyspark style return. This should not be a BCR because this map type is not publicly available yet.