[Enhancement] Support native MAP/STRUCT target columns for Avro routine load#74901
Conversation
aa6f59a to
d5b2127
Compare
b89345c to
2ea05e4
Compare
2ea05e4 to
59ae97b
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59ae97bb0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🌎 Translation Required?✅ All translation files are up to date.
|
…ne load Routine Load with format=avro aborted at BE scanner open() when a destination column was MAP or STRUCT, failing with "Not support cast VARCHAR(1048576) to MAP<...>". _construct_avro_types() only had a native ARRAY branch; MAP/STRUCT fell through to VARCHAR(MAX), and the subsequent VARCHAR->MAP/STRUCT cast does not exist. Build native MAP/STRUCT intermediate types and writers (mirroring the avro-cpp readers), and dispatch the column writer on the intermediate (_avro_types) type rather than the destination type so the source-column down_casts match. - avro_scanner: recursive construct_avro_type() for ARRAY/MAP/STRUCT; dispatch _construct_column on _avro_types in both the jsonpath and no-jsonpath paths (via a slot-id -> intermediate-type map). - formats/avro/nullable_column: native add_map_column/add_struct_column, union unwrapping for nested map values / struct fields / array elements, and snapshot/resize rewind on partial nested append. Avro map keys are always strings, so the intermediate map key is forced to VARCHAR/CHAR and the cast stage converts to the declared key type. The existing avro->JSON path (TYPE_JSON columns) is unchanged. Tests: native MAP/STRUCT, nested (ARRAY<MAP>, MAP<ARRAY>, ARRAY<STRUCT>, STRUCT-with-MAP, 4-level deep), nullable-union fields (null + value), both avro_ignore_union_type_tag values; regression test_map_to_json preserved. Docs: update the Avro -> StarRocks complex-type mapping table (maps -> MAP, record -> STRUCT) in loading/RoutineLoad.md (en/zh/ja). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
59ae97b to
c8fbc9d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8fbc9ddf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 128 / 152 (84.21%) file detail
|
|
do we by any chance overlap with #73821 ? |
@eshishki Yes, they have overlap. Functionally, this PR is indeed part of #73821, but there are the following differences:
|
|
@Mergifyio backport branch-4.1 |
✅ Backports have been createdDetails
Cherry-pick of 5e41562 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
…ne load (StarRocks#74901) Signed-off-by: PengFei Li <lpengfei2016@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Oliver Layer <o.layer@celonis.com>
…ibility test_struct_with_boolean depends on native STRUCT target support (PR StarRocks#74901, main only), so it cannot cherry-pick to release branches that lack it. Replace it with test_array_with_boolean, which drives the same add_nullable_column TYPE_BOOLEAN dispatch (the fixed line) and whose ARRAY path is present on all release branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GN8hHLLGz7o25mvSJhwqXr
Why I'm doing:
Routine Load with
format="avro"(Confluent Schema Registry) aborts at the BEscanner
open()when a destination column isMAPorSTRUCT, failing with:AvroScanner::_construct_avro_types()only had a native branch forARRAY;MAP/STRUCTfell through toVARCHAR(MAX), and the subsequentVARCHAR -> MAP/STRUCTcast does not exist, so the task fails before reading anydata. Scalars,
ARRAY, and nullable unions already worked; onlyMAP/STRUCT(and nesting that contains them) were broken.
What I'm doing:
Load avro
MAP/STRUCTnatively, with no JSON intermediary:avro_scanner.cpp— replace the ARRAY-only switch in_construct_avro_typeswith a recursiveconstruct_avro_type()that buildsnative
ARRAY/MAP/STRUCTintermediate load types (mirrorsJsonUtils::construct_json_type). Avro map keys are always strings, so theintermediate map key is forced to
VARCHAR/CHAR; the cast stage converts itto the declared key type.
_construct_columnnow dispatches the column writer on theintermediate (
_avro_types) type, which is what the source column was builtwith, instead of the destination type. Otherwise a complex column whose
intermediate type differs from the destination (e.g.
MAP<VARCHAR,VARCHAR>vsMAP<VARCHAR,DATE>) is written by a writer chosen for the destination andcrashes on
down_cast. Both call sites are fixed: the jsonpath path uses_avro_types[i]; the no-jsonpath path stores the intermediate type via a newslot-id -> type map.
formats/avro/nullable_column.cpp— nativeadd_map_column/add_struct_column, mirroring the avro-cppMapColumnReader/StructColumnReader:map key written with a length check + value recurses; struct fields matched by
name, missing fields become
NULL. Inner unions in nested map values / structfields / array elements are unwrapped, and a partial nested append is rewound
(snapshot +
resize) on error before decidingNULLvs. propagate.The existing avro -> JSON path (
TYPE_JSONcolumns) and scalar/array handling areunchanged. The cast factory already supports recursive
MAP->MAP/STRUCT->STRUCT/ARRAY->ARRAYcasts, so e.g.MAP<VARCHAR,DATE>works withoutany JSON round-trip.
Tests
AvroScannerTest,AvroAddNullableColumnTest): nativeMAP(with andwithout jsonpaths),
STRUCT(nullable-union field, null and non-null, bothavro_ignore_union_type_tagvalues), nestedARRAY<MAP>,MAP<ARRAY>,ARRAY<STRUCT>,STRUCT-with-MAP, and a 4-level-deepARRAY<STRUCT<VARCHAR, MAP<VARCHAR, ARRAY<INT>>>>; thetest_map_to_jsonregression is preserved.
MAP<VARCHAR(65533), INT>column loads native maps (map['k'],map_size,map_keys,map_valuesall work); previously it aborted atopen().python3 build-support/check_be_module_boundaries.py --mode fullclean.Docs
Updated the Avro → StarRocks complex-type mapping table in
loading/RoutineLoad.md(en/zh/ja):
maps→MAP or JSON,record→STRUCT, or ... JSON.Fixes #70928
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: