Rahul-cs50 opened a new pull request, #4264: URL: https://github.com/apache/iceberg-python/pull/4264
Closes #4191 # Rationale for this change `add_files` infers a file's partition from the parquet min and max of the source column. That ignores the null count. A file holding `'a'` and NULL has min = max = `'a'`, so it is registered under partition `'a'`. `s IS NULL` then prunes it, and the scan in the issue returns `[]` instead of `[2]`. A value and NULL are two partition values, just as `'a'` and `'b'` are. `add_files` already refuses a file holding `'a'` and `'b'`. This change refuses a file holding a value and NULL in the same way, using the null count that is already in `DataFileStatistics`. A file whose column is all NULL still goes to the NULL partition, as before. This PR covers only the value-and-NULL case. The issue mentions two other cases, and I left both alone: - **NaN next to a value.** Parquet statistics leave NaN out of min/max, and `nan_value_counts` is not filled from parquet metadata, so this check cannot see NaN. - **No column statistics** (`write.metadata.metrics.default=none`). The file gets a `None` partition, and `mkdocs/docs/api.md` documents that behavior. Changing it is a call for maintainers. ## Are these changes tested? Yes. `test_add_files_refuses_partition_value_with_nulls` in `tests/catalog/test_catalog_behaviors.py` runs on each catalog fixture: - It adds an all-`'a'` file and an all-NULL file, and checks that `s = 'a'` and `s IS NULL` return the right rows. - It then checks that a file holding `'a'` and NULL is refused and nothing is added. The test fails on `main` (no error is raised) and passes with this change. Unit tests (`tests/` without integration and cloud suites): 4495 passed; the 4 failures are Kerberos tests in `tests/catalog/test_hive.py` that fail the same way on `main` in my environment. `ruff check` and `ruff format --check` are clean. ## Are there any user-facing changes? Yes. `add_files` now raises `ValueError` for a file where an identity-partition source column, or one with an order-preserving transform, holds both values and NULLs. Before, such a file was silently given the wrong partition. -- 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]
