yihua commented on code in PR #13726:
URL: https://github.com/apache/hudi/pull/13726#discussion_r2283671005
##########
hudi-common/src/main/java/org/apache/hudi/avro/AvroRecordContext.java:
##########
@@ -181,4 +175,12 @@ public IndexedRecord toBinaryRow(Schema avroSchema,
IndexedRecord record) {
public UnaryOperator<IndexedRecord> projectRecord(Schema from, Schema to,
Map<String, String> renamedColumns) {
return record -> HoodieAvroUtils.rewriteRecordWithNewSchema(record, to,
renamedColumns);
}
+
+ @Override
+ public Comparable convertValueToEngineType(Comparable value) {
+ if (value instanceof Utf8) {
+ return ((Utf8) value).toString();
+ }
Review Comment:
Is there test coverage on this?
##########
hudi-common/src/main/java/org/apache/hudi/avro/AvroRecordContext.java:
##########
@@ -110,26 +105,25 @@ public HoodieRecord
constructHoodieRecord(BufferedRecord<IndexedRecord> buffered
HoodieKey hoodieKey = new HoodieKey(bufferedRecord.getRecordKey(),
partitionPath);
if (bufferedRecord.isDelete()) {
- if (payloadClass != null) {
- return SpillableMapUtils.generateEmptyPayload(
- bufferedRecord.getRecordKey(),
- partitionPath,
- bufferedRecord.getOrderingValue(),
- payloadClass,
- bufferedRecord.getHoodieOperation());
- } else {
- return new HoodieEmptyRecord<>(
- hoodieKey,
- bufferedRecord.getHoodieOperation(),
- OrderingValues.getDefault(),
- HoodieRecord.HoodieRecordType.AVRO);
- }
+ return generateEmptyAvroRecord(
+ hoodieKey,
+ bufferedRecord.getOrderingValue(),
+ payloadClass,
+ bufferedRecord.getHoodieOperation());
}
- if (requiresPayloadRecords) {
- HoodieRecordPayload payload =
HoodieRecordUtils.loadPayload(payloadClass, (GenericRecord)
bufferedRecord.getRecord(), bufferedRecord.getOrderingValue());
- return new HoodieAvroRecord<>(hoodieKey, payload,
bufferedRecord.getHoodieOperation(), bufferedRecord.isDelete());
+
+ return HoodieRecordUtils.createHoodieRecord((GenericRecord)
bufferedRecord.getRecord(), bufferedRecord.getOrderingValue(),
+ hoodieKey, payloadClass, bufferedRecord.getHoodieOperation(),
Option.empty(), false);
Review Comment:
nit: should new utils of `generateEmptyAvroRecord` and `createHoodieRecord`
taking `BufferedRecord` as the argument be created, which provide better
readability? Right now multiple fields are fetched from the `bufferedRecord`
and passed to the methods.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/SerializableIndexedRecord.java:
##########
@@ -98,6 +103,19 @@ public void read(Kryo kryo, Input input) {
this.recordBytes = input.readBytes(length);
}
+ private void writeObject(ObjectOutputStream out) throws IOException {
Review Comment:
If `Serializable` is supported, is `KryoSerializable` still needed?
--
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]
