-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-3921] Improve rewriteRecordWithNewSchema and refactor code #5393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@nsivabalan @alexeykudinkin could you pls help me review this pr, thanks |
| result = parentNames.stream().collect(Collectors.joining(".")); | ||
| } | ||
| return result; | ||
| return StreamSupport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for Stream you can just do String.join
|
@xiarixiaoyao can you please rebase and verify this patch via some testing? sorry about the delay. let's try to land this soon. |
| Map<Object, Object> map = (Map<Object, Object>) oldRecord; | ||
| Map<Object, Object> newMap = new HashMap<>(); | ||
| fieldNames.push("value"); | ||
| fieldNames.push(MAP_TYPE_VALUE_NAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the variable only used by this method, there is no need to make static variables, the JVM would cache the string constants.
|
@jonvex could you check if this PR is still needed? |
|
A lot of the changes in this pr are already in master. I think it's probably ok to close this |
fixed all comments on #5376 and optimize the implementation of rewriteRecordWithNewSchema