Skip to content

fix(ingestion): ingest STRUCT/ARRAY child columns for StarRocks and Doris - #36487

Open
TeddyCr wants to merge 1 commit into
open-metadata:mainfrom
TeddyCr:fix/starrocks-doris-struct-columns
Open

TeddyCr wants to merge 1 commit into
open-metadata:mainfrom
TeddyCr:fix/starrocks-doris-struct-columns

Conversation

@TeddyCr

@TeddyCr TeddyCr commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Description

The StarRocks and Doris connectors ingest nested columns (STRUCT, ARRAY<STRUCT>, MAP) without their child fields:

  • Show Full Columns returns the full type string (struct<latitude double, ...>), but the hand-rolled child parser in _get_column — a greedy regex child extraction plus a naive split(",")/split(":") — silently loses every struct field (StarRocks) and crashes on the missing field separator (Doris: the caught exception skips the column entirely). The ingested Column.children ends up empty, which blocks the nested-column DQ representation accepted by the backend since fix(dq): allow attaching test cases to nested columns #36385 (ADR:2026-10-27-nested-column-dq-tests-address-nested-paths).

Related: #36278 (epic), #36385 (backend slice — the API-side nested path resolution this unblocks end-to-end).

What changed

  • column_helpers.py (new shared home, alongside truncate_column_name):
    • parse_complex_column_type(type_str) — parses STRUCT/ARRAY/MAP type strings through the shared, bracket-aware ColumnTypeParser._parse_datatype_string (the same code path Trino/Databricks route through), returning the OM column dict with recursive children; soft-fails (warns, ingests the column without children) on unparsable types.
    • column_from_parsed(ordinal, parsed) — maps a parsed child dict onto the Column entity; deeper levels validate recursively.
    • Private _colonize_field_* helpers normalize field-name separators — StarRocks prints name type (space), Doris prints name:type (colon), and one form can appear nested inside the other; the split prefers whichever boundary is closer. Parameters like decimal(10, 2) are preserved via the bracket-aware split.
  • starrocks/metadata.py — _get_column now parses children for STRUCT and for array<struct> element fields; the custom get_columns_and_constraints override maps them through column_from_parsed.
  • doris/metadata.py — same shape; the previous naive split(":") could also IndexError/skip columns.
  • Top-level dataType/dataTypeDisplay are unchanged for both connectors (display keeps the raw source string verbatim).
  • ingestion/.basedpyright/baseline.json — down-shift only (entries removed in the two fixed files; no new baseline entries added).

No schema/migration change (Column.children already exists; BigQuery/Trino/Databricks already populate it).

How Has This Been Tested?

  • 24 new unit tests (test_starrocks_metadata.py, test_doris_metadata.py): 2-level nesting (struct field of struct), array<struct> children, map<...>, decimal(10,2) inside a struct field (precision/scale preserved), both separator forms, malformed type soft-fail, primitive types → no children, and children → Column recursion.
  • Regression: full ingestion/tests/unit/source/database/starrocks/ + .../doris/ suites (35 passed) and ingestion/tests/unit/topology/database/test_starrocks.py + test_doris.py (39 passed) — all green.
  • End-to-end check against a live StarRocks 4.0 stack: SHOW FULL COLUMNS type strings from a table with location STRUCT<...address STRUCT<...>> and vehicles ARRAY<STRUCT<...>> parse to 5 and 4 children respectively, with grandchildren on the nested struct.
  • ruff format / ruff check clean on all touched files; basedpyright 0 errors on the touched src files; pre-commit hooks pass.

Checklist

  • No schema change, no migration needed
  • Unit tests for the parsing/mapping; regression suites for both connectors
  • Connector-specific logic stays in connector files; shared, dialect-agnostic helpers centralized
  • .gitignore: ignore CLAUDE.local.md (local-only agent instructions file, prevents accidental commits)

…oris

The connectors hand-rolled nested-type parsing in _get_column for their
custom get_columns_and_constraints override: a greedy regex child
extraction plus a naive split on ','/':' silently lost struct fields
(StarRocks) and crashed on the missing field separator for Doris —
columns ingested without children.

Route STRUCT/ARRAY/MAP type strings through the shared bracket-aware
ColumnTypeParser via new shared helpers (parse_complex_column_type,
column_from_parsed) in column_helpers.py, normalizing the space
separated (StarRocks) and colon separated (Doris) field-name forms first,
with a soft-fail (warn + ingest without children) on unparsable types.

- column_helpers.py: shared parse_complex_column_type / column_from_parsed
- starrocks: children now parsed for structs and array<struct> elements
- doris: same; malformed types no longer crash (caught, skipped with warning)
- tests: 24 unit tests incl. 2-level nesting, decimal(10,2) inside a
  struct field, array<struct>, both separator forms, malformed input
- basedpyright baseline: down-shift only, no entries added
Copilot AI balanced review requested due to automatic review settings October 28, 2026 21:49
@TeddyCr
TeddyCr requested a review from a team as a code owner October 28, 2026 21:50
@TeddyCr
TeddyCr requested a review from ulixius9 October 28, 2026 21:50
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions github-actions Bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Oct 28, 2026
@gitar-bot

gitar-bot Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Code Review ✅ Approved

🟡 Medium risk · Parsing nested StarRocks and Doris types changes catalog column metadata; incorrect parsing could misrepresent child fields and affect downstream nested-column queries.

Adds recursive child-column parsing for STRUCT, ARRAY<STRUCT>, and MAP types in the StarRocks and Doris connectors, so nested fields are ingested into Column.children instead of being dropped. No issues found.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingestion safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants