danny0405 commented on code in PR #19858:
URL: https://github.com/apache/hudi/pull/19858#discussion_r3949381432
##########
hudi-common/src/main/java/org/apache/hudi/common/engine/RecordContext.java:
##########
@@ -440,11 +441,8 @@ private SerializableBiFunction<T, HoodieSchema, String>
metadataKeyExtractor() {
return (record, schema) -> getValue(record, schema,
RECORD_KEY_METADATA_FIELD).toString();
}
- private SerializableBiFunction<T, HoodieSchema, String>
virtualKeyExtractor(String[] recordKeyFields) {
- if (recordKeyFields.length == 1) {
- // there might be consistency for record key encoding when partition
fields are multiple for cow merging,
- // currently the incoming records are using the keys from HoodieRecord
which utilities the write config and by default encodes the field name with the
value
- // while here the field names are ignored, this function would be used
to extract record keys from old base file.
+ private SerializableBiFunction<T, HoodieSchema, String>
virtualKeyExtractor(String[] recordKeyFields, int numPartitionFields) {
+ if (recordKeyFields.length == 1 && numPartitionFields <= 1) {
Review Comment:
You're right: CustomAvroKeyGenerator delegates a single record key to
SimpleAvroKeyGenerator regardless of the partition-field count. From the code,
the partition-count change therefore introduces a mismatch for that Spark
combination. We're keeping the broader key-generator alignment work separate
from this PR; this case remains unresolved here. Consulting the table's
generator class alone is not sufficient for all engines, since Flink generates
incoming keys through RowDataKeyGen. Any follow-up needs to align extraction
with each writer without taking the read behavior from the write config.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieConcatHandle.java:
##########
@@ -93,8 +94,8 @@ public HoodieConcatHandle(HoodieWriteConfig config, String
instantTime, HoodieTa
*/
@Override
public void write(HoodieRecord oldRecord) {
- HoodieSchema oldSchema = config.populateMetaFields() ?
writeSchemaWithMetaFields : writeSchema;
- String key = oldRecord.getRecordKey(oldSchema, keyGeneratorOpt);
+ HoodieSchema oldSchema = config.getMetaFieldsMode() == MetaFieldsMode.NONE
? writeSchema : writeSchemaWithMetaFields;
Review Comment:
Fixed in ea4cbcb61660: the schema choice now uses the inherited
metaFieldsMode, resolved from the table config, consistently with the file-name
rewrite. All four Spark concat metadata-mode cases passed locally.
##########
hudi-client/hudi-flink-client/src/main/java/org/apache/hudi/io/storage/row/HoodieRowDataLanceWriter.java:
##########
@@ -118,12 +119,20 @@ public void writeRow(String key, RowData row) throws
IOException {
@Override
public void writeRowWithMetaData(HoodieKey key, RowData row) throws
IOException {
- if (populateMetaFields) {
- RowData rowWithMeta = updateRecordMetadata(row, key,
getWrittenRecordCount());
- writeRow(key.getRecordKey(), rowWithMeta);
+ RowData rowWithMeta;
Review Comment:
Keeping the branches explicit in each writer for this PR. We considered the
shared-helper approach and chose to leave HoodieRowDataCreation unchanged: each
caller handles ALL and NONE directly and passes the selected metadata values to
the existing creation API. The duplication is intentional here; no performance
improvement is claimed without benchmarking.
--
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]
