voonhous commented on code in PR #19809:
URL: https://github.com/apache/hudi/pull/19809#discussion_r3916656149
##########
hudi-common/src/main/java/org/apache/hudi/common/avro/HoodieAvroUtils.java:
##########
@@ -883,15 +924,16 @@ private static Object
normalizeAvroLogicalTypeToPrimitive(Object value, Schema s
*
* @param record Hoodie record.
* @param columns Names of the columns to get values.
- * @param schema {@link Schema} instance.
+ * @param schema {@link HoodieSchema} instance.
* @return Column value.
*/
public static Object[] getRecordColumnValues(HoodieRecord record,
String[] columns,
- Schema schema,
+ HoodieSchema schema,
boolean
consistentLogicalTimestampEnabled) {
try {
- GenericRecord genericRecord = (GenericRecord)
(record.toIndexedRecord(HoodieAvroSchemaCache.intern(schema), new
Properties()).get()).getData();
+ // Intern so the identity fast path in BaseAvroPayload#getRecord hits
across callers that parse their own copy of the schema.
+ GenericRecord genericRecord = (GenericRecord)
(record.toIndexedRecord(HoodieSchemaCache.intern(schema), new
Properties()).get()).getData();
Review Comment:
Done in 1125fa3541c9: `HoodieAvroSchemaCache.intern(schema.toAvroSchema())`,
the master path, with the comment reworded to describe the cache choice.
##########
hudi-common/src/test/java/org/apache/hudi/common/avro/TestHoodieAvroUtils.java:
##########
@@ -955,6 +1085,25 @@ void
testGetSortColumnValuesWithPartitionPathAndRecordKey(boolean suffixRecordKe
}
}
+ @Test
+ void testGetRecordColumnValues() {
Review Comment:
Done in 1125fa3541c9: `testGetRecordColumnValuesInternsSchema` builds the
record on the interned Avro schema under `OverwriteWithLatestAvroPayload` and
reads it with a freshly parsed `HoodieSchema`. Checked that it fails without
the intern (`expected: <String> but was: <Utf8>`). `student.firstnameNested`
dropped from the other test.
--
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]
