yihua commented on PR #20029:
URL: https://github.com/apache/hudi/pull/20029#issuecomment-5784221538
Suggested direction for this PR: **return the base file format's short name
from `shortName()` (`parquet` / `orc` via `tableConfig.getBaseFileFormat`),
drop the optimizer rule, and keep the test but assert on the nested predicates
themselves.**
Why this is preferable to `hoodiefilegroup` + conf injection:
- **Covers every read path.** The conf injection only runs in sessions that
load `HoodieSparkSessionExtension`. `spark.read.format("hudi")`, catalog reads
without `spark.sql.extensions`, Spark Connect and Thrift Server deployments
stay regressed. A base-format short name matches Spark's default `parquet,orc`
allowlist everywhere, with zero user action.
- **No session-state mutation from the planner.** Optimizer rules are
expected to be pure; `spark.conf.set` from inside a fixed-point batch is
surprising, and the one-shot `applied` flag means a later `RESET` /
`spark.conf.unset` switches pushdown off for the rest of the session.
- **Restores prior behaviour rather than inventing new.** 0.x
(`Spark3xLegacyHoodieParquetFileFormat`) and 1.0.x
(`HoodieFileGroupReaderBasedParquetFileFormat`) both inherited `parquet`;
`HudiFileGroup` only arrived with HUDI-8746 (#13632). Users migrating from
those versions get the same `Format: parquet` in EXPLAIN they had before.
- **ORC is handled by the same change.** Both `ParquetFilters` and
`OrcFilters` resolve nested (quoted multipart) column names, and the allowlist
only gates translation, so `parquet` / `orc` per base format is enough; for
multi-format tables returning `parquet` is fine for the same reason.
- **One thing to keep in sync instead of two.** With `hoodiefilegroup` the
short name and the injected conf value must agree forever; with the base-format
name there is one string, and a small unit test pinning `shortName()` per base
format prevents the next accidental rename.
- **No change to the other Spark gates.** `SchemaPruning` and
`PushVariantIntoScan` match on `isInstanceOf[ParquetFileFormat]`, which is
unaffected either way.
The cost is losing the Hudi-specific string in the scan node's `Format`;
`HoodieFileIndex` in the same node still identifies the table as Hudi, so I
think that is acceptable.
--
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]
