hudi-agent commented on code in PR #19247:
URL: https://github.com/apache/hudi/pull/19247#discussion_r3565746177
##########
rfc/rfc-103/rfc-103.md:
##########
@@ -257,9 +259,11 @@ Using native log file format can also be extended to other
file format, like [La
### Block type handling
**Data log**: The Parquet schema is the writer's table schema, including Hudi
metadata columns (`_hoodie_commit_time`, `_hoodie_commit_seqno`,
`_hoodie_record_key`, `_hoodie_partition_path`, `_hoodie_file_name`), followed
by the user-defined data columns.
-The schema is stored natively in the Parquet footer (no duplication with
header metadata). The `hudi.log.format.metadata` footer entry carries the
serialized metadata map as described above.
+The physical schema is stored natively in the Parquet footer. The
`hudi.log.format.SCHEMA` entry
Review Comment:
🤖 This paragraph now says the physical schema lives natively in the Parquet
footer while `hudi.log.format.SCHEMA` separately preserves the Hudi logical
schema — but the previous version of this section explicitly stated the schema
was stored natively with "no duplication with header metadata". Could you
clarify the rationale for now carrying two schema representations, which one is
the source of truth on read, and how you prevent the native and logical schemas
from drifting?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
rfc/rfc-103/rfc-103.md:
##########
@@ -223,19 +223,21 @@ Footer
- key-value metadata <-- Hudi log format metadata goes in here
```
-Hudi log format metadata can be stored as a single entry in the Parquet footer
with key = `hudi.log.format.metadata` and value being a serialized JSON of the
metadata entries:
-
-| Hudi log format metadata |
-|:---------------------------------------------|
-| `VERSION` |
-| `INSTANT_TIME` |
-| `TARGET_INSTANT_TIME` |
-| `COMMAND_BLOCK_TYPE` |
-| `COMPACTED_BLOCK_TIMES` |
-| `RECORD_POSITIONS` |
-| `BLOCK_IDENTIFIER` |
-| `IS_PARTIAL` |
-| `BASE_FILE_INSTANT_TIME_OF_RECORD_POSITIONS` |
+Each Hudi log format metadata value is stored as a separate Parquet footer
entry under the
+`hudi.log.format.` namespace:
+
+| Hudi log format metadata | Parquet footer key
|
+|:---------------------------------------------|:---------------------------------------------------------------|
+| `VERSION` | `hudi.log.format.VERSION`
|
+| `INSTANT_TIME` |
`hudi.log.format.INSTANT_TIME` |
+| `TARGET_INSTANT_TIME` |
`hudi.log.format.TARGET_INSTANT_TIME` |
+| `SCHEMA` | `hudi.log.format.SCHEMA`
|
+| `COMMAND_BLOCK_TYPE` |
`hudi.log.format.COMMAND_BLOCK_TYPE` |
+| `COMPACTED_BLOCK_TIMES` |
`hudi.log.format.COMPACTED_BLOCK_TIMES` |
Review Comment:
🤖 The PR description notes this is a breaking on-disk change — files written
with the old `hudi.log.format.metadata` JSON entry are no longer readable
through the new path — but the RFC itself doesn't state a compatibility stance.
Could you add a note on whether RFC-103 native logs are still considered
experimental/unreleased (so no migration is needed) or whether existing files
require an upgrade/rewrite path?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/util/FileFormatUtils.java:
##########
@@ -231,6 +231,18 @@ public String[] readMinMaxRecordKeys(HoodieStorage
storage, StoragePath filePath
public abstract Map<String, String> readFooter(HoodieStorage storage,
boolean required, StoragePath filePath,
String... footerNames);
+ /**
+ * Read data file footer entries whose keys start with the given prefix.
+ *
+ * @param storage {@link HoodieStorage} instance
+ * @param required require at least one matching footer entry
+ * @param filePath data file path
+ * @param footerPrefix prefix of the footer entries to read
+ * @return matching footer entries keyed by their full names
+ */
Review Comment:
🤖 `readFooterWithPrefix` is added as an `abstract` method, which forces
every current and third-party `FileFormatUtils` implementation to provide it or
fail to compile. Would a default method that throws
`UnsupportedOperationException` (mirroring the Vortex behavior) be a gentler
extension point for the shared abstraction? @bvaradar for a read on the
API-compatibility expectations here.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
