-
Notifications
You must be signed in to change notification settings - Fork 2
Added support for add column and drop column #41
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
base: master
Are you sure you want to change the base?
Conversation
Types.NestedField field; | ||
|
||
switch (type.toLowerCase()) { | ||
case "string": |
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.
couldn't find any mapper from string to iceberg Types
return new ColumnSpec(columnName, field.type(), comment); | ||
} | ||
|
||
private static Types.NestedField parseColumnType(String name, String type, String 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.
maybe this needs to be moved to a common class for field mapping.
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.
I think we should go with @aadant's proposal #39 (comment). Separate add-column/delete-column commands instead of single alter-table means you can't combine multiple changes in a single transaction.
Invalid operation
|
names = {"--operations"}, | ||
required = true, | ||
description = | ||
"JSON array of operations: [{'add': 'age:int:User age'}, {'drop': 'col_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.
It would be best to stick to yaml/json for this as column names may contain colons, future operations may not be so easy to express in colon-separated values, etc.
It would also be nice if operation/files names would follow https://iceberg.apache.org/docs/latest/spark-ddl/#alter-table-rename-column naming.
closes: #39