prantogg opened a new pull request, #842: URL: https://github.com/apache/sedona-db/pull/842
## Summary - PR #646 ([Issue #250](https://github.com/apache/sedona-db/issues/250)) wired the file metadata cache through the GeoParquet wrapper's own `DFParquetMetadata` calls (schema inference, wrapper pruning fetch) — but not through the inner DataFusion `ParquetOpener`, which uses its own `ParquetFileReaderFactory`. With the inner factory left at the default `DefaultParquetFileReaderFactory`, the inner opener's `ArrowReaderMetadata::load_async` fetches the parquet footer fresh from the object store on every query. SQL queries against a registered view kept paying the full `metadata_load_time` (~30s on Overture-scale remote reads) even with `datafusion.runtime.metadata_cache_limit` configured and warm. - Fix: install `CachedParquetFileReaderFactory` on the inner `ParquetSource` in `GeoParquetFormat::create_physical_plan`, mirroring DataFusion's stock `ParquetFormat::create_physical_plan`. Adds one `with_parquet_file_reader_factory` passthrough on `GeoParquetFileSource` matching the existing `with_metadata_size_hint` shape. This also addresses the unresolved symptom yutannihilation reported on PR #646 ("Even if I include `SET datafusion.runtime.metadata_cache_limit = '900M'`, the benchmark still doesn't improve much") — same root cause. ## Test plan - [x] New unit test `create_physical_plan_installs_cached_reader_factory` builds a plan via `GeoParquetFormat::create_physical_plan` and structurally asserts the inner `ParquetSource` has a custom `ParquetFileReaderFactory`. Confirmed to fail on `main` (`got None`) and pass with this change. - [x] `cargo test -p sedona-geoparquet --lib` — 61 passed, 0 failed. - [x] `cargo clippy -p sedona-geoparquet --lib --tests` — clean on changed files (only pre-existing warnings in `statistics_accumulator.rs`). - [ ] End-to-end verification against the Overture `division_area` reproducer from the bug report: subsequent SQL queries should see `metadata_load_time` drop to near-zero on the 2nd+ execution. ## Follow-up note (not in this PR) Noticed while tracing: `GeoParquetFileSource::try_from_file_source` always sets `metadata_cache: None` and relies on every caller to manually re-set it (e.g., `updated_inner.metadata_cache = self.metadata_cache.clone();` in `try_pushdown_filters`, `with_batch_size`, etc.). Easy to forget when adding a new caller. The factory introduced here doesn't have this footgun — it rides on the inner `ParquetSource` and is preserved by `ParquetSource::clone`. Worth a separate small refactor. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
