Skip to content

RawEdge.kind duplicates the column's own kind everywhere but the set-operation redirect #27

Description

@funcpp

Found while reviewing #13 (#25).

RawEdge.kind is a copy of the output column's kind

There are seven edge-creation sites. Six of them — build/select.rs ×3, build/statement.rs ×3 — are this shape:

let kind = classify_expr(expr);
let output = self.graph.add_output(name.clone(), kind.clone());   // on the node
for &anc in &ancestors {
    self.graph.add_edge(anc, output, kind.clone());               // and on every edge
}

The seventh is the set-operation redirect in build/query.rs, which copies the other branch's edges onto this column and so is the one place an edge carries a kind the node does not already hold.

Nothing outside derive_transform reads RawEdge.kind — not the DOT output, not the JSON, not the Python bridge.

Measured on 9d8636f: classifying from the column's own kind alone, ignoring edge_kinds entirely, passes the whole suite and changes exactly one of 42 query shapes (SELECT a FROM t UNION ALL SELECT SUM(b) FROM u, which needs the redirected branch). That is the entire information content of the field.

Removing it would also fix a case that is still wrong

SELECT a FROM t UNION ALL SELECT COUNT(*) FROM u
a  ←  t.a    (direct)

COUNT(*) touches no column, so the right branch has no edge to redirect and its aggregation reaches the left column by no route at all. #25 fixed the mirror image of this (SELECT COUNT(*) ... UNION ALL SELECT a ...) because there the aggregate branch was the one that owns the node. This direction needs the merge to happen between nodes, not between edges.

So: drop kind from RawEdge, and have the set-operation redirect merge the right branch's node kind into the left's. derive_transform then takes one kind and becomes a plain mapping into TransformKind, collect_output_sources stops returning a kind vector, and ~14 EdgeKind clones go away.

Related question

EdgeKind and TransformKind are near-isomorphic — Direct/ViaExpression/ViaAggregation/ViaConditional against Direct/Expression/Aggregation/Conditional, with TransformKind adding Window and Unknown, neither of which is produced anywhere. If the enum stops labelling edges, the case for keeping two of them is thin. Worth deciding together with the above, though collapsing them would put a public type in the internal graph.

Depends on #26 — both touch how kinds travel through the walk, and that one is the correctness bug of the two. This is in the area #14 reworks, so it may be resolved there.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions