yihua commented on code in PR #677:
URL: https://github.com/apache/hudi-rs/pull/677#discussion_r3770373033
##########
crates/core/src/file_group/reader.rs:
##########
@@ -221,6 +222,88 @@ impl FileGroupReader {
.await
}
+ /// Which merge implementation serves this read.
+ ///
+ /// A metadata table is always served by the legacy reader whatever the
+ /// setting says: its base files and log blocks are HFile, which the
+ /// merge-on-read engine has no support for. That is permanent, not
+ /// transitional.
+ ///
+ /// The value is read raw rather than through `get_or_default`, which falls
+ /// back to the default when a value fails to parse. A typo in the engine
+ /// name would then silently read with the other engine, leaving a caller
+ /// convinced they had exercised it — the one outcome this switch must not
+ /// produce.
+ fn file_group_reader_version(&self) -> Result<FileGroupReaderVersion> {
+ if self.is_metadata_table() {
+ return Ok(FileGroupReaderVersion::One);
+ }
+ match self
+ .hudi_configs
+ .as_options()
+ .get(HudiReadConfig::FileGroupReaderVersion.as_ref())
+ {
+ Some(raw) =>
FileGroupReaderVersion::from_str(raw).map_err(CoreError::Config),
+ None => Ok(FileGroupReaderVersion::default()),
+ }
+ }
+
+ /// Why the merge-on-read engine cannot serve this read, if it cannot.
+ ///
+ /// This is a capability check, decided from config before any I/O — never
a
+ /// catch-all on error. A read that fails *inside* the engine propagates:
+ /// retrying it on the legacy reader would make a bug look like a success,
+ /// make results depend on which engine happened to win, and leave the
+ /// differential tests unable to see anything.
+ ///
+ /// Every reason here means the legacy reader serves the read instead, so
+ /// selecting the engine cannot turn a working read into a failing one.
Each
+ /// reason is logged, because a fallback nobody can observe is
+ /// indistinguishable from an engine that is never used.
+ fn version_two_unsupported_reason(
+ &self,
+ options: &ReadOptions,
+ ) -> Result<Option<&'static str>> {
+ // Deliberately an error rather than a fallback: falling back would use
+ // the legacy reader's own merge derivation, which drops deletes on a
+ // commit-time-ordered table. Wrong rows are worse than a refusal.
+ if let Some(mode) = self
+ .hudi_configs
+ .as_options()
+ // Read by raw key: this crate has no typed config for it yet, and
+ // adding one belongs with the reader that acts on it.
+ .get("hoodie.record.merge.mode")
+ && mode.eq_ignore_ascii_case("CUSTOM")
+ {
+ return Err(CoreError::Unsupported(
Review Comment:
Let's add a regression test around this.
--
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]
