-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-3522] Introduce DropColumnSchemaPostProcessor to support drop columns from schema #4972
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
|
@hudi-bot run azure |
|
hi @XuQianJin-Stars, could you please help review this as well ? |
...-utilities/src/main/java/org/apache/hudi/utilities/schema/DropColumnSchemaPostProcessor.java
Outdated
Show resolved
Hide resolved
...-utilities/src/main/java/org/apache/hudi/utilities/schema/DropColumnSchemaPostProcessor.java
Outdated
Show resolved
Hide resolved
...-utilities/src/main/java/org/apache/hudi/utilities/schema/DropColumnSchemaPostProcessor.java
Outdated
Show resolved
Hide resolved
pratyakshsharma
left a comment
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.
Changes look good to me overall. Minor comments. Please address them.
@pratyakshsharma thanks for you review, I have addressed all your concern, please take another look when free |
|
Thank you for another useful processor @wangxianghu . Will merge once the CI passes. |
|
@hudi-bot run azure |
1 similar comment
|
@hudi-bot run azure |
| List<Schema.Field> targetFields = new LinkedList<>(); | ||
|
|
||
| for (Schema.Field sourceField : sourceFields) { | ||
| if (!columnsToDelete.contains(sourceField.name().toLowerCase(Locale.ROOT))) { |
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.
Avro schema fields are actually case sensitive. I was reading about this today. But in practical world, any table should not have column names which differ only in case sensitivity. So this conversion to lower case should not break anything.
…olumns from schema (apache#4972) * [HUDI-3522] Introduce DropColumnSchemaPostProcessor to support drop columns from schema * Fix case sensitivity
…olumns from schema (apache#4972) * [HUDI-3522] Introduce DropColumnSchemaPostProcessor to support drop columns from schema * Fix case sensitivity
…olumns from schema
Tips
What is the purpose of the pull request
Introduce DropColumnSchemaPostProcessor to support drop columns from schema
Verify this pull request
This pull request is already covered by existing tests, such as (please describe tests).
org.apache.hudi.utilities.TestSchemaPostProcessor#testDeleteColumn
org.apache.hudi.utilities.TestSchemaPostProcessor#testDeleteColumnThrows
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green
Necessary doc changes done or have another open PR
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.