partition_filter on s3.read_parquet_table ignores last partition

[vlieven]

Describe the bug

When using the s3.read_parquet_table function, the partition filter will work for the first n-1 partitions, but the final nth partition will not be present in the dictionary consumed by the filtering function.

I've investigated the issue, and I believe the culprit is line 73 in _extract_partitions_from_path in s3._read:
path_wo_filename: str = path.rpartition("/")[0] + "/"

If the path value is not a filename, but rather a directory that does not end in "/", this will remove the nth partition from the path.

Environment

awswrangler = 2.13.0

To Reproduce

This happens when the partition locations returned by AWS Glue do not end in "/".
We are writing these tables using Apache Spark, which might be causing this.

To Solve

Based on the documentation for the get_partitions function in catalog._get, I think the cleanest fix for this would be to ensure that the partition locations retrieved from AWS Glue effectively end in "/" and add the missing slash if not.
I will create a PR for this fix.

[kukushking]

Thanks @vlieven, indeed we rely on the convention that folders/partitions end with /. I'm a bit hesitant on the proposed fix though because there are cases where the Glue table is just a single parquet file, for example. Simply adding a / to the locations would break these use cases.

[vlieven]

I see... Well there are some alternatives I think:

• If you assume that a valid parquet partition should either end in "/" or ".parquet", you can append a missing "/" based on that convention.
• If a partition can be anything, you could use the partition values returned by Glue to verify if the partitions that were extracted from the path match those stored in Glue. If not, something probably went wrong, and we can retry by adding a "/" to the path. That would mean extending the _apply_partition_filter function to optionally accept a List[List[str]] for the expected values, and use that for verification if supplied.
• The final thing I can think of is to query the S3 API to check whether a URI is a prefix or an object, and use that to repair any URI that does not follow the convention. Although I would like to avoid this solution if possible.

Would any of these be acceptable solutions? I'm willing to work on a new implementation if you can point me in the direction that you think is best

[kukushking]

Glue partitions generally follow a=b convention - perhaps if there is no / at the end we could check the tail against that pattern, and if it's a match do consider that a partition, too. That would also be in line with how _extract_partitions_from_path extracts partition keys and values after the prefix is stripped off so it would definitely be a safe bet.

Happy to accept the contribution!

[vlieven]

Ok, I've updated the PR with the same checking logic as used by extract_partitions_from_path.
If you think it would be better, we could also check this through a regex.

[kukushking]

Looking great, thanks for the contribution!