nsivabalan commented on code in PR #19205:
URL: https://github.com/apache/hudi/pull/19205#discussion_r3531454370
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/row/HoodieRowCreateHandle.java:
##########
@@ -160,11 +170,45 @@ public HoodieRowCreateHandle(HoodieTable table,
public void write(InternalRow row) throws IOException {
if (populateMetaFields) {
writeRow(row);
+ } else if (populateCommitTime || populateFileName) {
+ writeRowSelectiveMetaFields(row);
} else {
writeRowNoMetaFields(row);
}
}
+ /**
+ * Selective meta-field write path: populate only the meta columns opted in
via
+ * {@code hoodie.meta.fields.mode} — {@code _hoodie_commit_time} (with
derived seq id) and/or
+ * {@code _hoodie_file_name}. The other meta columns stay null on disk.
Record key is never
+ * populated in this path, so the record key is not registered with the
write support (bloom
+ * filter / RLI hooks are meaningless without the record-key column).
+ */
+ private void writeRowSelectiveMetaFields(InternalRow row) {
+ try {
+ UTF8String[] metaFields = new UTF8String[5];
+ if (populateCommitTime) {
+ metaFields[HoodieRecord.COMMIT_TIME_METADATA_FIELD_ORD] =
shouldPreserveHoodieMetadata
+ ? row.getUTF8String(HoodieRecord.COMMIT_TIME_METADATA_FIELD_ORD) :
commitTime;
+ metaFields[HoodieRecord.COMMIT_SEQNO_METADATA_FIELD_ORD] =
shouldPreserveHoodieMetadata
Review Comment:
why did we include COMMIT_SEQNO_METADATA_FIELD_ORD also here?
we wanted only the commit time meta field.. since thats whats is required
for incremental queries.
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/io/storage/HoodieSparkParquetWriter.java:
##########
@@ -74,6 +94,16 @@ public void writeRowWithMetadata(HoodieKey key, InternalRow
row) throws IOExcept
super.write(row);
writeSupport.add(recordKey);
+ } else if (populateCommitTime || populateFileName) {
+ if (populateCommitTime) {
+ row.update(COMMIT_TIME_METADATA_FIELD.ordinal(), instantTime);
+ row.update(COMMIT_SEQNO_METADATA_FIELD.ordinal(),
Review Comment:
same comment as above.
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroParquetWriter.java:
##########
@@ -72,6 +94,20 @@ public void writeAvroWithMetadata(HoodieKey key,
IndexedRecord avroRecord) throw
taskContextSupplier.getPartitionIdSupplier().get(),
getWrittenRecordCount(), fileName);
super.write(avroRecord);
writeSupport.add(key.getRecordKey());
+ } else if (populateCommitTime || populateFileName) {
+ // Selective meta-field population. The other meta columns stay null on
disk, which Parquet
+ // stores as definition-level flags (zero data bytes). Bloom filter /
record-key index
+ // population is intentionally skipped — that requires the record-key
column.
+ GenericRecord genericRecord = (GenericRecord) avroRecord;
+ if (populateCommitTime) {
+ String seqId = HoodieRecord.generateSequenceId(instantTime,
Review Comment:
we don't need to add commit seq no
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroFileWriterFactory.java:
##########
@@ -89,7 +89,7 @@ protected HoodieFileWriter newParquetFileWriter(
hoodieConfig.getLongOrDefault(HoodieStorageConfig.PARQUET_MAX_FILE_SIZE),
storageConfiguration,
hoodieConfig.getDoubleOrDefault(HoodieStorageConfig.PARQUET_COMPRESSION_RATIO_FRACTION),
hoodieConfig.getBooleanOrDefault(HoodieStorageConfig.PARQUET_DICTIONARY_ENABLED));
- return new HoodieAvroParquetWriter(path, parquetConfig, instantTime,
taskContextSupplier, populateMetaFields);
+ return new HoodieAvroParquetWriter(path, parquetConfig, instantTime,
taskContextSupplier, populateMetaFields, metaFieldsMode);
Review Comment:
can we not fold both `populateMetaFields` arg and `metaFieldsMode` into one
argument. do we really need both?
not just here. in all writer factory and other places, where we are adding
`metaFieldsMode` as new arg in addition to `populateMetaFields`.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/IncrementalRelationV1.scala:
##########
@@ -89,8 +88,9 @@ class IncrementalRelationV1(val sqlContext: SQLContext,
s"option ${DataSourceReadOptions.START_COMMIT.key}")
}
- if (!metaClient.getTableConfig.populateMetaFields()) {
- throw new HoodieException("Incremental queries are not supported when meta
fields are disabled")
+ if (!metaClient.getTableConfig.isCommitTimePopulated()) {
+ throw new HoodieException("Incremental queries are not supported when
_hoodie_commit_time is not populated. "
+ + "Either keep hoodie.populate.meta.fields=true or include
_hoodie_commit_time in hoodie.meta.fields.mode.")
Review Comment:
lets also call out that, a table once disabled for commit time, can never be
switched back. So, users got to re-create a new table.
just setting these writer props during next write may not suffice.
lets capture this in the msg.
--
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]
