xushiyan commented on code in PR #10337:
URL: https://github.com/apache/hudi/pull/10337#discussion_r1429556607
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:
##########
@@ -251,9 +305,14 @@ private static <R> Option<HoodieRecord<R>>
mergeIncomingWithExistingRecord(
HoodieRecord<R> existing,
Schema writeSchema,
HoodieWriteConfig config,
- HoodieRecordMerger recordMerger) throws IOException {
+ HoodieRecordMerger recordMerger,
+ BaseKeyGenerator keyGenerator) throws IOException {
Schema existingSchema = HoodieAvroUtils.addMetadataFields(new
Schema.Parser().parse(config.getSchema()),
config.allowOperationMetadataField());
Schema writeSchemaWithMetaFields =
HoodieAvroUtils.addMetadataFields(writeSchema,
config.allowOperationMetadataField());
+ if (keyGenerator != null) {
+ return mergeIncomingWithExistingRecordWithExpressionPayload(incoming,
existing, writeSchema,
+ existingSchema, writeSchemaWithMetaFields, config, recordMerger,
keyGenerator);
+ }
Review Comment:
not quite sure about this dependency: keygen not null then go with
expression payload.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:
##########
@@ -243,6 +247,56 @@ private static <R> HoodieData<HoodieRecord<R>>
getExistingRecords(
.getMergedRecords().iterator());
}
+ /**
+ * getExistingRecords will create records with expression payload so we
overwrite the config.
+ * Additionally, we don't want to restore this value because the write will
fail later on.
+ * We also need the keygenerator so we can figure out the partition path
after expression payload
+ * evaluates the merge.
+ */
+ private static BaseKeyGenerator
maybeGetKeygenAndUpdatePayload(HoodieWriteConfig config) {
+ if
(config.getPayloadClass().equals("org.apache.spark.sql.hudi.command.payload.ExpressionPayload"))
{
+ config.setValue(HoodiePayloadConfig.PAYLOAD_CLASS_NAME.key(),
HoodiePayloadConfig.PAYLOAD_CLASS_NAME.defaultValue());
+ try {
+ return (BaseKeyGenerator)
HoodieAvroKeyGeneratorFactory.createKeyGenerator(config.getProps());
+ } catch (IOException e) {
+ throw new RuntimeException("KeyGenerator must inherit from
BaseKeyGenerator to update a records partition path using spark sql merge
into", e);
+ }
+ }
+ return null;
Review Comment:
let's avoid `null` as return value - at least you can wrap it with `Option`
--
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]
