Fix multi hive-partition parquet reading in dask-cudf

[rjzamora]

This PR fixes some un-tested hive-partitioning edge cases in read-parquet. The "full" fix requires dask#8072. However, this PR should be merged before the upstream change goes in (otherwise dask_cudf.read_parquet CI will temporarily break). Note that this change is non-breaking, but the corresponding dask-dataframe change is.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.10@4d8e401). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 7742364 differs from pull request most recent head f9968a7. Consider uploading reports for the commit f9968a7 to get more accurate results

@@               Coverage Diff               @@
##             branch-21.10    #9122   +/-   ##
===============================================
  Coverage                ?   10.87%           
===============================================
  Files                   ?      115           
  Lines                   ?    19141           
  Branches                ?        0           
===============================================
  Hits                    ?     2082           
  Misses                  ?    17059           
  Partials                ?        0           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d8e401...f9968a7. Read the comment docs.

[galipremsagar]

@rjzamora Changes look good to me, will there be a followup PR after dask/dask#8072 goes in? If not we would probably have to tag this as a breaking change so that this makes into the breaking change CHANGELOG.md entry.

[rjzamora]

Changes look good to me, will there be a followup PR after dask/dask#8072 goes in? If not we would probably have to tag this as a breaking change so that this makes into the breaking change CHANGELOG.md entry.

Thanks for the review @galipremsagar! Are you refering to the fact that dask#8072 is technically a breaking change without this PR in place (even though this PR is not "breaking")? It is true that cudf<=21.08 will run into dask-cudf test failures with dask versions released after dask#8072 is merged.

[galipremsagar]

Are you refering to the fact that dask#8072 is technically a breaking change without this PR in place (even though this PR is not "breaking")?

Yes, exactly.

It is true that cudf<=21.08 will run into dask-cudf test failures with dask versions released after dask#8072 is merged.

I see, we don't guarantee cudf<=21.08 will work with a dask version which is above the max pinned version for that release. But that said, will the end-user have to make changes to their code while using read_parquet after this PR & dask#8072 are merged? Seems like the function signature change in dask upstream PR is breaking change?

[rjzamora]

But that said, will the end-user have to make changes to their code while using read_parquet after this PR & dask#8072 are merged? Seems like the function signature change in dask upstream PR is breaking change?

This is a good question. We are refactoring code and tweaking some function signatures, but the intention was to avoid removing kwargs from any public functions. Therefore, the down-stream user should not need to change any code after these PRs go in. However, if the user is on dask>=2021.9.0 (assuming the dask PR gets merged for that release), then they will want to use cudf>=21.10. The parquet API will mostly work for people with older cudf versions, but hive-partitioned columns will not be detected in some cases (i.e. they may have missing columns with some settings).

I'm certainly not completely sure of the "correct" way to label this PR, so I'm happy to defer to you :)

[galipremsagar]

The parquet API will mostly work for people with older cudf versions, but hive-partitioned columns will not be detected in some cases (i.e. they may have missing columns with some settings).

Okay, got it. Then lets keep the tags as is. Thanks for explaining this to me @rjzamora 🙏

[rjzamora]

rerun tests

[quasiben]

@gpucibot merge