L4XB opened a new pull request, #45617:
URL: https://github.com/apache/superset/pull/45617
### SUMMARY
A columnar (Parquet) upload accepts a ZIP of several files.
`ColumnarReader._yield_files` collected the suffix of every name in the archive
and refused the upload when it found more than one, and then read every name as
a Parquet file. A ZIP made from a folder holds more names than its data files,
so these uploads failed:
- `zip -r sales.zip sales` writes a folder entry `sales/`, whose suffix is
empty.
- On macOS, `ditto -c -k --sequesterRsrc --keepParent sales sales.zip` also
writes `__MACOSX/` folders and an AppleDouble `__MACOSX/sales/._<name>` entry
next to each file with extended attributes.
- A folder can also hold `.DS_Store`, or marker files such as Spark's
`_SUCCESS`.
The two archives, built from a folder with two Parquet files:
```
zip -r: ['sales/', 'sales/a.parquet', 'sales/b.parquet']
ditto: ['sales/', 'sales/a.parquet', '__MACOSX/', '__MACOSX/sales/',
'__MACOSX/sales/._a.parquet',
'sales/b.parquet', '__MACOSX/sales/._b.parquet']
```
On `master`, both fail with `ZIP file contains multiple file types`, in
`file_to_dataframe` (the upload) and in `file_metadata` (the upload metadata
endpoint). With this PR, both read the three rows of the two files.
Changes:
- `_yield_files` reads only data entries. It skips folder entries and names
that start with `.` or `_`. pyarrow's dataset discovery applies the same rule
by default (`ignore_prefixes=['.', '_']`), and superset's import bundles skip
the same names (`is_valid_config` in `superset/commands/importers/v1/utils.py`).
- The "multiple file types" check looks at the data entries only. A ZIP that
mixes Parquet files with other data files is still refused.
- A ZIP without any data entry now fails with `ZIP file contains no data
files`. On `master`, a ZIP that holds only a folder entry failed with `Parsing
error: ... Parquet file size is 0 bytes`, because the folder entry was read as
a file, and an empty ZIP raised an unhandled `ValueError: No objects to
concatenate` from `pd.concat`.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/commands/databases/columnar_reader_test.py`: 17
passed.
- New `test_columnar_reader_zip_of_a_folder`,
`test_columnar_reader_zip_of_a_folder_metadata` and
`test_columnar_reader_zip_without_data_files` fail on `master`. The archive
they use has folder entries, `__MACOSX` AppleDouble files, `.DS_Store` and
`_SUCCESS` around two Parquet files.
- `pytest tests/unit_tests/databases/api_test.py -k "columnar or upload"`:
90 passed.
- `ruff check` / `ruff format` (0.9.7) are clean, and `pylint` rates the
file 10.00/10. `mypy --check-untyped-defs` reports one message in
`columnar_reader.py`, on the unchanged `yield file` line (`FileStorage` vs
`IO[bytes]`). `master` reports the same message.
To check by hand, put two Parquet files with the same columns in a folder,
run `zip -r folder.zip folder`, and upload `folder.zip` as a columnar file. On
this branch both files load. On `master` the upload fails with `ZIP file
contains multiple file types`.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
