nsivabalan commented on code in PR #8573:
URL: https://github.com/apache/hudi/pull/8573#discussion_r1181990009
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecord.java:
##########
@@ -358,7 +358,7 @@ public final void read(Kryo kryo, Input input) {
*
* NOTE: This operation is idempotent
*/
- public abstract HoodieRecord prependMetaFields(Schema recordSchema, Schema
targetSchema, MetadataValues metadataValues, Properties props);
+ public abstract Option<HoodieRecord> prependMetaFields(Schema recordSchema,
Schema targetSchema, MetadataValues metadataValues, Properties props);
Review Comment:
this is a public interface. lets add a old one back and mark it as
deprecated.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieAvroRecord.java:
##########
@@ -126,12 +126,16 @@ public HoodieRecord joinWith(HoodieRecord other, Schema
targetSchema) {
}
@Override
- public HoodieRecord prependMetaFields(Schema recordSchema, Schema
targetSchema, MetadataValues metadataValues, Properties props) {
+ public Option<HoodieRecord> prependMetaFields(Schema recordSchema, Schema
targetSchema, MetadataValues metadataValues, Properties props) {
try {
Option<IndexedRecord> avroRecordOpt =
getData().getInsertValue(recordSchema, props);
- GenericRecord newAvroRecord =
HoodieAvroUtils.rewriteRecordWithNewSchema(avroRecordOpt.get(), targetSchema);
- updateMetadataValuesInternal(newAvroRecord, metadataValues);
- return new HoodieAvroRecord<>(getKey(), new
RewriteAvroPayload(newAvroRecord), getOperation(), this.currentLocation,
this.newLocation);
+ if (avroRecordOpt.isPresent()) {
Review Comment:
can you help me understand which flow/use-case will hit this. From what I
gleaned, most of the callers to this method already filter for deleted record
and should not be calling this.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieAvroRecord.java:
##########
@@ -126,12 +126,16 @@ public HoodieRecord joinWith(HoodieRecord other, Schema
targetSchema) {
}
@Override
- public HoodieRecord prependMetaFields(Schema recordSchema, Schema
targetSchema, MetadataValues metadataValues, Properties props) {
+ public Option<HoodieRecord> prependMetaFields(Schema recordSchema, Schema
targetSchema, MetadataValues metadataValues, Properties props) {
try {
Option<IndexedRecord> avroRecordOpt =
getData().getInsertValue(recordSchema, props);
- GenericRecord newAvroRecord =
HoodieAvroUtils.rewriteRecordWithNewSchema(avroRecordOpt.get(), targetSchema);
- updateMetadataValuesInternal(newAvroRecord, metadataValues);
- return new HoodieAvroRecord<>(getKey(), new
RewriteAvroPayload(newAvroRecord), getOperation(), this.currentLocation,
this.newLocation);
+ if (avroRecordOpt.isPresent()) {
Review Comment:
so, this is the main fix in this patch is it ?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieAppendHandle.java:
##########
@@ -249,13 +249,13 @@ private Option<HoodieRecord>
prepareRecord(HoodieRecord<T> hoodieRecord) {
// Prepend meta-fields into the record
MetadataValues metadataValues = populateMetadataFields(finalRecord);
- HoodieRecord populatedRecord =
+ Option<HoodieRecord> populatedRecord =
Review Comment:
for records that are deleted, isn't finalRecordOpt.isPresent() will be false
and hence the condition in L243 will be false and we will not hit this
condition only.
--
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]
