feat: Upgrade iceberg-java to 1.9.2 and support adding required columns#145
feat: Upgrade iceberg-java to 1.9.2 and support adding required columns#145subkanthi wants to merge 17 commits into
Conversation
…port for alter table add column with default, non/null columns.
…hub.com/Altinity/ice into iceberg_update_alter_table_null_default
…with required flag
…hub.com/Altinity/ice into iceberg_update_alter_table_null_default
…_update_alter_table_null_default
xieandrew
left a comment
There was a problem hiding this comment.
We need to address how to handle adding a required column on a table with existing data in AlterTable. Currently this leads to an invalid table.
| } | ||
|
|
||
| String valuesSql = | ||
| "SELECT b_int, b_string, toString(b_date), formatDateTime(b_ts, '%Y-%m-%d %H:%m:%S') FROM `" |
There was a problem hiding this comment.
formatDateTime is using the month %m twice instead of %M for the minute. It passes because the expected value below also has the month as the incorrect minute value.
| if (!"2024-06-15".equals(cells[2])) { | ||
| throw new AssertionError("b_date: expected 2024-06-15, got " + cells[2]); | ||
| } | ||
| if (!"2024-06-15 12:06:45".equals(cells[3])) { |
There was a problem hiding this comment.
After changing the formatDateTime in the SQL above, the time should be 12:30:45 to match the parquet input.
| // primitives (date/time/timestamp/decimal/uuid/binary) we accept a string form and let | ||
| // Iceberg parse it via Literal<CharSequence>.to(type). | ||
| @Nullable | ||
| private static Literal<?> coerceDefault(Type type, @Nullable Object jsonValue) { |
There was a problem hiding this comment.
This method isn't called anywhere. What is its use?
| // Iceberg rejects required adds without an initial default unless incompatible | ||
| // changes are explicitly allowed (even for empty tables with a snapshot). | ||
| us.allowIncompatibleChanges(); |
There was a problem hiding this comment.
I think this leads to an invalid state when you add a required column when the table is not empty. There's no initial default value being used, so this violates the required constraint. Doing ice scan on the table results in java.lang.IllegalArgumentException: Missing required field: extra at org.apache.iceberg.data.parquet.BaseParquetReaders$ReadBuilder.defaultReader(BaseParquetReaders.java:271) for the iris table.
closes: #144