924060929 commented on PR #64446: URL: https://github.com/apache/doris/pull/64446#issuecomment-4978612479
Follow-up on the `file_format` per-split note from my earlier review, re-checked against the current branch tip (the `FIX-JNI-FILE-FORMAT (P7-1)` version of `PaimonScanPlanProvider`). **Credit first**: the P7-1 fix is a real correctness fix — emitting the literal `"jni"` as `fileDesc.file_format` broke `paimon_cpp_reader`'s FILE_FORMAT/MANIFEST_FORMAT backfill, and the comment there confirms the field is genuinely load-bearing for the cpp reader (not informational). Good catch. **The remaining gap**: after the fix, `buildJniScanRange` and `buildCountRange` set `.fileFormat(defaultFileFormat)` — the **table-level** `file.format` option (default `parquet`) — while `buildNativeRange` derives it **per data file** via `getFileFormatBySuffix(file.path()).orElse(defaultFileFormat)`. So the per-split slot that `PaimonScanRange.fileFormat` provides is filled per-file on the native path but collapsed to a single table-level value on the JNI / COUNT-pushdown paths. **Blast radius is genuinely narrow for paimon** (unlike iceberg, where per-file format is first-class): `file.format` is normally authoritative because paimon writes all data files in the configured format, and the native path is already per-file-correct. The residual mis-read cases are tables whose on-disk data-file format diverges from the table option — e.g. `file.format` changed after data was already written (mixed old/new files), or per-level formats — read via the JNI/cpp path. **One-line alignment** if you want to close it: reuse the native path's per-file derivation in the two JNI/COUNT builders — `getFileFormatBySuffix(<data-file-path>).orElse(defaultFileFormat)` instead of the bare `defaultFileFormat`. Not blocking — flagging so it's a conscious call rather than an oversight, given P7-1 already touched exactly this line. *Structural note (about the SPI, not this PR): the abstraction itself is correctly two-level — `ConnectorTableSchema.tableFormatType` = `"paimon"`/`"iceberg"` (table-level) vs `ConnectorScanRange.getFileFormat()` = per-split file format — so this is a connector-fill discipline issue, not an SPI-shape flaw. The SPI just doesn't* enforce *per-split, which is what lets a code path quietly collapse to a table-level value.* -- 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]
