yihua commented on code in PR #12122:
URL: https://github.com/apache/hudi/pull/12122#discussion_r1819894815
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/BaseHoodieMergedLogRecordScanner.java:
##########
@@ -87,6 +94,11 @@ protected BaseHoodieMergedLogRecordScanner(HoodieStorage
storage, String basePat
this.records = new ExternalSpillableMap<>(maxMemorySizeInBytes,
spillableMapBasePath, new DefaultSizeEstimator(),
new HoodieRecordSizeEstimator(readerSchema), diskMapType,
isBitCaskDiskMapCompressionEnabled);
this.scannedPrefixes = new HashSet<>();
+
+ // TODO: use reader context for both non fg reader.
+ this.orderingFieldName =
Option.ofNullable(ConfigUtils.getOrderingField(payloadProps)).orElseGet(() ->
hoodieTableMetaClient.getTableConfig().getPreCombineField());
+ this.orderingFieldType =
AvroSchemaUtils.findNestedFieldType(readerSchema,
this.orderingFieldName).orElse(Schema.Type.INT);
+ this.orderingFieldDefault = TypeCaster.castValue(0, orderingFieldType);
Review Comment:
Default should always be integer-typed 0 for backwards compatibility.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/BaseHoodieMergedLogRecordScanner.java:
##########
@@ -217,11 +229,15 @@ protected void processNextDeletedRecord(DeleteRecord
deleteRecord) {
// Put the DELETE record
if (recordType == HoodieRecord.HoodieRecordType.AVRO) {
records.put((K) key, SpillableMapUtils.generateEmptyPayload(key,
- deleteRecord.getPartitionPath(), deleteRecord.getOrderingValue(),
getPayloadClassFQN()));
+ deleteRecord.getPartitionPath(), deleteOrderingVal,
getPayloadClassFQN()));
} else {
- HoodieEmptyRecord record = new HoodieEmptyRecord<>(new HoodieKey(key,
deleteRecord.getPartitionPath()), null, deleteRecord.getOrderingValue(),
recordType);
+ HoodieEmptyRecord record = new HoodieEmptyRecord<>(new HoodieKey(key,
deleteRecord.getPartitionPath()), null, deleteOrderingVal, recordType);
records.put((K) key, record);
}
+
+ if (deleteRecord.getOrderingValue() == null) {
+ records.get((K)
key).addMetadata(HoodieReaderContext.DELETE_FOUND_WITHOUT_ORDERING_VALUE,
"true");
+ }
Review Comment:
If we always assume that `(integer) 0` represents natural ordering, is
`DELETE_FOUND_WITHOUT_ORDERING_VALUE` still necessary?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/BaseHoodieMergedLogRecordScanner.java:
##########
@@ -87,6 +94,11 @@ protected BaseHoodieMergedLogRecordScanner(HoodieStorage
storage, String basePat
this.records = new ExternalSpillableMap<>(maxMemorySizeInBytes,
spillableMapBasePath, new DefaultSizeEstimator(),
new HoodieRecordSizeEstimator(readerSchema), diskMapType,
isBitCaskDiskMapCompressionEnabled);
this.scannedPrefixes = new HashSet<>();
+
+ // TODO: use reader context for both non fg reader.
+ this.orderingFieldName =
Option.ofNullable(ConfigUtils.getOrderingField(payloadProps)).orElseGet(() ->
hoodieTableMetaClient.getTableConfig().getPreCombineField());
+ this.orderingFieldType =
AvroSchemaUtils.findNestedFieldType(readerSchema,
this.orderingFieldName).orElse(Schema.Type.INT);
Review Comment:
If ordering field is not set, make this `Option.empty()` instead of assuming
a default type.
--
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]
