Summary coverage: positional column identity and a lazy coverage cache (follow-up to #646) #651

Follow-up to the two review comments of #646 from 2026-10-10. Both are fixed in #646 (4083c643) with the simplest change; this issue records the alternatives in case we need them later.

1. Column identity in a selection

Current (#646, #573 §4.2.2): a selection column is (table, name) in the schema of the SummaryAgg's child. A Filter/TimeRange/TimeShift that renames a column is not removed from the definition, so these names are always the names in the definition's child. The cost: such a node lifts no conditions.

Alternative (2), closer to Goldstein & Larson: identify a selection column by its position (ColumnId) in the definition child's output, and use the name only for display. A merge requires equal definitions, so the same position means the same column, just as G&L identify view columns by base-table columns that the view definition fixes. With this:

• renames never matter;
• the ambiguous-name check (Project [a AS k, b AS k]) can be removed.

Cost: ColumnIdentity changes type, the §4.2.2 text changes, and the e2e assertions change.

2. Caching coverage()

Current (#646): there is no cache. coverage() derives on each call and returns an owned value. A cache inside the node went stale when a node was edited in place, because operator and schema are public (e.g. through Rc::make_mut). Recomputing costs O(chain length) per SummaryAgg and O(subtree) per merge. validate_structure over nested merges costs nodes × depth.

If profiling shows this matters, there are two lazy options that stay correct:

• a. Make the IR truly immutable. Make operator and schema private, with getters plus operator_mut()/schema_mut() that clear the cache.
  • Why it is correct: coverage reads only operator/schema of a node and its descendants. A descendant is behind an Rc, so editing it in place needs the ancestor's operator_mut(), which clears every ancestor on the path. A shared Rc is cloned by make_mut, and the clone starts empty.
  • Cost: about 620 field reads and 13 in-place edits at the top of the stack become method calls, which is a separate refactor.
• b. External memo. A pass (e.g. validate_structure, the search space) keeps HashMap<*const OperatorNode, (Rc<OperatorNode>, SummaryCoverage)>.
  • Why it is correct: holding the Rc keeps the strong count ≥ 2, so the node cannot be edited in place (get_mut fails and make_mut clones), and its address cannot be reused.
  • Cost: no API change, but coverage() alone stays uncached.

Checking a cached value for staleness is not an option: the check walks the same subtree as deriving the coverage again.