Conversation
|
Create table in ice CH WRITE PATH |
xieandrew
left a comment
There was a problem hiding this comment.
An edge case and potential optimization I found, other than that it looks good.
| auto inner_type = removeNullable(sample_block->getByPosition(i).type); | ||
| if (isNothing(inner_type)) |
There was a problem hiding this comment.
It looks like this doesn't check for Nothing type inside nested types (tuple, array, or map), so those nested Nothing values are not filtered for the writer. It would be good to check if that causes the parquet writer to fail.
| filtered_columns.reserve(columns.size() - nothing_column_indices.size()); | ||
| for (size_t i = 0; i < columns.size(); ++i) | ||
| { | ||
| if (std::find(nothing_column_indices.begin(), nothing_column_indices.end(), i) == nothing_column_indices.end()) |
There was a problem hiding this comment.
The std::find on every column/chunk could removed if the constructor computes a list of column indices to keep instead of nothing_column_indices. Then you would only need to iterate the kept column indices and directly add columns[i] to filtered_columns.
Audit update for PR #2363 (Iceberg v3
|
| /// Iceberg schema metadata and is read back as NULLs on the read path. | ||
| for (size_t i = 0; i < sample_block->columns(); ++i) | ||
| { | ||
| if (!containsNothing(sample_block->getByPosition(i).type)) |
There was a problem hiding this comment.
It looks like this excludes the entire column from being written, even if other nested fields are not the Nothing type. Only the unknown field in the nested should be stripped otherwise there might be data loss.
There was a problem hiding this comment.
yes its this AI finding #2363 (comment)
|
|
||
| # This must NOT fail with UNKNOWN_TYPE. | ||
| instance.query( | ||
| f"INSERT INTO {table_name} (id, name) VALUES (3, 'charlie'), (4, 'dave')", |
There was a problem hiding this comment.
This insert should write the struct containing the unknown e.g. (42, NULL) and the next SELECT should check that the non-unknown nested field is properly written.
|
I don't think these are mentioned here anywhere. Can you please check? Medium: Promoting Iceberg v3 allows Medium:
Medium: An insert into a table that has a top-level
Medium: A write that strips every column produces a Parquet file the reader rejects. If every top-level column contains |
…n column will be ignored in file stats, inserts with nothing will be rejected
…ty/ClickHouse into iceberg_unknown_data_type
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Support for iceberg v3
unknowndatatype which is maps toNullable(Nothing)in Clickhouse. Read and write path(Parquet).CI/CD Options
Exclude tests:
Regression jobs to run: