hudi-agent commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3802624638
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/parquet/io/HoodieParquetBinaryCopyBase.java:
##########
@@ -119,6 +120,20 @@ public abstract class HoodieParquetBinaryCopyBase
implements Closeable {
// Flag to control schema evolution behavior
protected Boolean schemaEvolutionEnabled = null;
+ /**
+ * Whether {@code _hoodie_file_name} is populated on this table.
+ *
+ * <p>Binary copy rewrites the column to the output file name, which is
right for a table that
+ * populates it and wrong for one that does not -- clustering would hand
such a table a file name it
+ * never advertised. Defaults to {@code ALL} so callers that do not state a
mode keep the historical
+ * behaviour.
+ */
+ private MetaFieldsMode metaFieldsMode = MetaFieldsMode.ALL;
+
+ public void setMetaFieldsMode(MetaFieldsMode metaFieldsMode) {
Review Comment:
🤖 nit: could you add `import java.util.Objects;` and use
`Objects.requireNonNull(...)` here? The fully-qualified form is a bit jarring
when the rest of the new code in this file uses short names.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/MergeOnReadIncrementalRelationV1.scala:
##########
@@ -261,8 +261,19 @@ trait HoodieIncrementalRelationV1Trait extends
HoodieBaseRelation {
s"option ${DataSourceReadOptions.START_COMMIT.key}")
}
- if (!this.tableConfig.populateMetaFields()) {
- throw new HoodieException("Incremental queries are not supported when
meta fields are disabled")
+ // MoR incremental relies on _hoodie_commit_time being present in BOTH
base files AND log
+ // records. The base-file writer respects hoodie.meta.fields.mode, but the
log-write path
+ // (HoodieAppendHandle) does not yet — until that gap is closed, MoR
incremental must require
+ // populate.meta.fields=true to avoid silently dropping log-file rows
whose commit_time is null.
+ //
+ // Gate on the table type, not just the flag:
HoodieCopyOnWriteIncrementalHadoopFsRelationFactory
+ // builds one of these relations for CoW tables too (it backs
HoodieIncrementalFileIndex), so an
+ // unconditional check here rejects every selective-mode CoW table — the
case this release
+ // actually supports. CoW keeps its own commit-time guard in
IncrementalRelationV1/V2.
+ if (metaClient.getTableType == HoodieTableType.MERGE_ON_READ &&
!this.tableConfig.populateMetaFields()) {
Review Comment:
🤖 I traced the routing and want to double-check the CoW commit-time-absent
case. `HoodieCopyOnWriteIncrementalHadoopFsRelationFactoryV1/V2` construct this
very relation for CoW datasource incremental reads, while
`IncrementalRelationV1/V2` (which the comment says guards CoW) are only
constructed by the streaming sources. So for a CoW `FILE_NAME_ONLY`/`NONE`
table this `validate()` is now skipped, yet the range filter still emits
`IsNotNull(_hoodie_commit_time)` over an all-null column — which would silently
return zero rows rather than error. Where does the datasource-incremental path
actually reject a commit-time-absent CoW table? Would
`!isCommitTimePopulated()` (matching how `IncrementalRelationV1/V2` were
relaxed) be the correct condition here instead of gating on table type?
<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]
