Skip to content

migrate-dataform: convert ARRAY<STRUCT> rather than only flagging it #54

Description

@ihistand

Landed as analysis + recommendation + SQL (3f35b54, 498f575). Auto-rewriting is deliberately not part of it; reasoning below.

What the converter does now

findStructArrays locates ARRAY(SELECT AS STRUCT …) specifically — an ARRAY of scalars unnests to a plain column and needs none of this — and extracts the column, the struct's fields, the source relation, and the join key from the inner query's correlation predicate.

chooseStrategies then decides from how the column is actually read across the project:

condition strategy
every read is an UNNEST collapse — the array is a round trip
nothing reads it collapse (drop) — confirm, then delete rather than port
a reader does more than unnest it child-table
jsonb stays the fallback, never the default

The report carries both the rationale and the SQL, with real names substituted:

`definitions/eav.sqlx` L5 — collapse — every read of `decimals` is an UNNEST
(1 file(s)), so the array is a round trip

    -- Drop the `decimals` column, and have each reader pivot raw_decimal
    -- directly. A consumer that today reads:
    --   (select any_value(value) from unnest(decimals) where key = '<name>')
    -- becomes, selecting from raw_decimal grouped by row_id:
    max(value) filter (where key = '<name>') as <name>

The join key matters more than it looks: where d.row_id = p.row_id is what says this array is the rows of d belonging to each row of p. A recommendation that omits it leaves the reader to find it, which is the slow part.

Why the rewrite is not automatic

Collapse deletes a column and rewrites its consumers; child-table adds an action to the graph and changes the parent's schema. Both restructure the model rather than translating syntax, and this issue's own argument was that such a transformation has to be legible. Emitting a recommendation a person can read, check and apply satisfies that; silently doing it does not.

Whether to automate the collapse case — the one where the graph proves the array is dead weight — is worth deciding against a real project rather than in the abstract. Tracked with the rest of the validation in the follow-up issue.

Bugs found on the way, both by running the flow rather than reading it

  • The analysis ran before buildTriage, which rebuilds report.todo from findings and discarded the entry. Silent — the report simply looked unchanged.
  • x as "key" was being rewritten to x as 'key' by the double-quote rule added in d1489dd. After AS a double-quoted token is an ALIAS, not a string; converting it is invalid SQL anywhere and quietly turns a column name into a literal, in the position a reader is least likely to check.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions