[python] Parquet read path #407
Conversation
python/iceberg/api/file_format.py
Outdated
| ORC = "orc" | ||
| PARQUET = "parquet" | ||
| AVRO = "avro" | ||
| ORC = "orc", True |
There was a problem hiding this comment.
It might be more clear to use a dictionary here to make it clear what these values mean:
e.g.:
ORC = dict(extension = "orc", splittable=True)
PARQUET = dict(extension = "parquet", splittable=True)
AVRO = dict(extension = "avro", splittable=True)
I had to read through the code to understand what these meant.
There was a problem hiding this comment.
All right fixed the test and lint errors. I agree that it's better to be explicit about the enum attributes.
There was a problem hiding this comment.
I also added a test for file format just for good measure
| processing_func = process_funcs.get(field.type.type_id) | ||
|
|
||
| if processing_func is None: | ||
| try: |
There was a problem hiding this comment.
I'm not clear on how we are dealing with values that are not in the avro record here. In the case that an optional field is not in the avro record, we should emit "null", but it appears that we always fail.
There was a problem hiding this comment.
Let me think through this. I had refactored this code and I think I just left out the .is_required check on that raise.
There was a problem hiding this comment.
I added in the is_required check to the raise and also tweaked the get map function to follow the pattern of the rest of the functions eg use bracket with a try block instead of get.
I'm going to write up some tests for the avro module to explicitly test some of these cases. Not sure if you want to wait on the merge for this or not
TGooch44
force-pushed
the
parquet_read_path_1
branch
5 times, most recently
from
4fcf04a
to
5f71879
Compare
TGooch44
force-pushed
the
parquet_read_path_1
branch
from
5f71879
to
57721d7
Compare
+1 LGTM. Thanks Ted
First part of commit to add in read support for parquet format.