parallel-workload: Check stalls#35750
Draft
def- wants to merge 11 commits into
Draft
Conversation
Contributor
|
Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone. PR title guidelines
Pre-merge checklist
|
8d73f63 to
bef3a21
Compare
Contributor
Author
|
Status: blocked by https://github.com/MaterializeInc/database-issues/issues/11305 |
Contributor
Author
|
Ready for review. New test run: https://buildkite.com/materialize/nightly/builds/16376 Edit: Unexpectedly there's a bunch of new failures, have to. check in with Dov. |
The reader for range columns called `push_range_with`, which writes the range to the row verbatim. For discrete range types (int4/int8/date), Parquet files authored by external engines may encode ranges with non-canonical bounds — e.g. `[1,10]` for `int4range`, which MZ stores internally as `[1,11)`. Without canonicalization those rows do not compare or hash equal to logically-identical values constructed inside MZ, so `COPY FROM PARQUET` rows mismatch their pure-SQL counterparts. Switch the decode path to `push_range`, which canonicalizes the range before packing. Add a unit test that decodes a hand-built non-canonical arrow StructArray and a testdrive regression test that round-trips a DuckDB-authored Parquet file through `COPY FROM ... (FORMAT PARQUET)`. Fixes DB-55. https://claude.ai/code/session_01KXioUsT2r5o2tRwrsHm4iR
Contributor
Author
Three runtime errors surface when a parallel-workload run drives an Iceberg sink with a wider mix of column types: 1. "Datum Int16 does not match builder Int32Builder" — Iceberg has no smallint, so the writer context arrow schema (derived from the iceberg schema) uses Int32 while Materialize rows still carry Datum::Int16. Add a lossless Int16 -> Int32 promotion in ArrowColumn::append_datum, mirroring the existing UInt16 -> Int32 case. 2. "Field 'value' missing extension metadata" — Materialize names map fields entries/keys/values, but iceberg-rust's arrow conversion names them key_value/key/value. merge_field_metadata_recursive matched by name and silently dropped the value field's extension metadata, so ArrowBuilder later failed when constructing the inner builder. Match the map entries struct positionally instead. 3. "Failed to create EqualityDeleteWriterConfig: field_id N not found" — the planner accepted Range types as Iceberg equality delete keys, but ranges lower into Iceberg structs and iceberg-rust's RecordBatchProjector skips nested fields, so the equality field id is unreachable at runtime. Drop Range from the allow-list so the failure is caught at sink creation instead.
Explain why positional matching in `merge_map_entries_metadata` is correct by citing the Arrow `Schema.fbs` definition of `Map` as `List<entries: Struct<key, value>>` with non-enforced field names.
Pins the three runtime failures fixed in the previous commit: - `merge_map_entries_preserves_value_extension_metadata`: unit test that builds a materialize-shaped map field (entries/keys/values) and an iceberg-shaped one (key_value/key/value) and asserts the merge copies the value field's extension metadata positionally. - `test/iceberg/key-validation.td`: adds a Range key rejection block alongside the existing Map/List rejections. - `test/iceberg/catalog.td`: adds smallint and map[text=>text] sinks exercising the Int16->Int32 promotion and the entries-struct metadata merge end-to-end against a real Iceberg catalog.
DuckDB's iceberg_scan returns 0 rows for map-valued tables in the versions we test against, so the round-trip via map_keys/map_values was not actually exercising the metadata merge — the assertion just failed with no actionable signal. Check mz_sink_statuses for `running` instead: without `merge_map_entries_metadata` the sink stalls with "Field 'value' missing extension metadata" during ArrowBuilder construction, which is exactly the regression we want to pin.
builder_for_datatype was hard-coding MapFieldNames::default() (entries/keys/values) when constructing the MapBuilder, regardless of what the surrounding Schema actually said. For Iceberg the schema's map fields are key_value/key/value (preserved by merge_map_entries_metadata), so the resulting MapArray's nested DataType disagreed with the schema and RecordBatch::try_new rejected every row — the sink stalled silently and iceberg_scan saw an empty table. Read entry/key/value names off the schema's entries struct so the MapArray matches whichever convention the caller chose. COPY TO S3 PARQUET keeps building its schema with the arrow-rs defaults, so its output is unchanged. Also restores the DuckDB iceberg_scan assertion on the map_table sink in catalog.td now that the round-trip actually works.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Inspired by https://materializeinc.slack.com/archives/CU7ELJ6E9/p1774556502630089
We should have had this originally!