Skip to content

Conversation

@flashJd
Copy link
Contributor

@flashJd flashJd commented Aug 12, 2022

Change Logs

fix primary key extract of delete_record when complexKeyGen configured and ChangeLogDisabled
In this condition, primary key in RowData will be like "uuid:id1", not "id1";

Impact

when complexKeyGen configured and ChangeLogDisabled, the primary key output is not correct

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@flashJd
Copy link
Contributor Author

flashJd commented Aug 12, 2022

@garyli1019 @danny0405 can you help review it, thx

@XuQianJin-Stars
Copy link
Contributor

@hudi-bot run azure

}

return Arrays.stream(fieldKV).map(kv -> {
final String[] kvArray = kv.split(":");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, why a simple key uses the key form: name:val instead of just val ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, why a simple key uses the key form: name:val instead of just val ?

options.put(FlinkOptions.RECORD_KEY_FIELD.key(), "uuid");
options.put(FlinkOptions.PARTITION_PATH_FIELD.key(), "partition,name");
options.put(FlinkOptions.KEYGEN_TYPE.key(), KeyGeneratorType.COMPLEX.name());
If pk is "uuid", partition is "partition,name", in flink-sql we'll use COMPLEX KeyGenerator, then uuid will stored as "uuid:danny", I've tested it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danny0405 I've explained it, looking forward to your reply.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why we configure a COMPLEX key generator while the key is just simple here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why we configure a COMPLEX key generator while the key is just simple here?
Because flink-sql's default logic is COMPLEX KeyGenerator, when boolean complexHoodieKey = pks.length > 1 || partitions.length > 1;
https://github.com/apache/hudi/blob/master/hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/table/HoodieTableFactory.java#L239

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, i have merged #6539 , so this pr can be closed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, i have merged #6539 , so this pr can be closed.

@danny0405 #6539 has little problem, if it's single pk and simple key generator, we'll store 'danny' not 'id:danny', so kvArray[1] will be null point

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, we can rebase the PR and fix it.

Copy link
Contributor Author

@flashJd flashJd Sep 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danny0405 , I've rebased this PR to master to fix the issue.
By the way, can you review #6429, thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed it in master, so this PR can be closed, would review #6429 then.

@hudi-bot
Copy link
Collaborator

hudi-bot commented Sep 5, 2022

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants