fix: qualify bare Column expressions in denormalized UNION branches

[genezhang]

Summary

• Denormalized ViewScans produce bare Column("OriginCityName") expressions without table qualifiers, causing the SQL renderer to guess t as the alias instead of the correct FROM alias (e.g., r)
• Two-level fix: qualify Column → PropertyAccessExp in plan_builder.rs (GraphNode UNION branches) and to_sql_query.rs (catch-all before SQL rendering)
• Promotes 3 denormalized simple_agg tests from xfail to passing (132 passed, 32 skipped, 40 xfailed)

Test plan

• All 1588 Rust tests pass
• cargo fmt --all and cargo clippy --all-targets clean
• Integration test suite: 132 passed, 32 skipped, 40 xfailed (no regressions)
• Verify no LDBC regressions (ClickHouse was down during testing, but sql_only mode confirmed correct SQL generation)

🤖 Generated with Claude Code

[genezhang]

Code Review

Concern: Duplicate qualification logic

The same bare-Column → PropertyAccessExp transformation is applied in two places:

1. plan_builder.rs:1676-1689 — qualifies UNION branch SELECTs at render plan construction time, using gn.alias
2. to_sql_query.rs:2699-2710 — qualifies UNION branch SELECTs again at SQL render time, using plan.from.alias

If fix #1 works correctly, fix #2 will find no bare Column expressions left to qualify (it becomes a no-op). Conversely, if #1 misses some cases, #2 acts as a safety net. This is the stated intent ("two-level fix"), but it creates a maintenance concern:

• Divergent alias sources: plan_builder.rs uses gn.alias (the GraphNode alias), while to_sql_query.rs uses plan.from.alias (the FROM clause alias). These should always agree for well-formed plans, but if they ever diverge, one fix would silently mask the other, making debugging harder.
• Redundant code: The inline loop in plan_builder.rs duplicates the logic of qualify_bare_columns_from_alias(). Consider extracting to a shared helper, or better yet, pick one canonical location and remove the other.

Suggestion: The plan_builder.rs fix (at plan construction time) is the more principled location — it fixes the data model rather than patching at render time. Consider keeping only that one and removing the to_sql_query.rs catch-all, or at minimum adding a comment in each location cross-referencing the other.

The fix itself is correct ✅

The core logic is sound — bare Column("OriginCityName") expressions from denormalized ViewScans need table qualification to avoid the t alias heuristic. Converting to PropertyAccessExp(alias, col) is the right approach.

Test changes ✅

The 3 simple_agg xfail removals are directly validated by the fix. Results (132 passed, 32 skipped, 40 xfailed) show continued progress.

Minor nit

qualify_bare_columns_from_alias uses unwrap_or_default() on a missing FROM alias and returns early on empty string. This is fine as a guard, but a log::debug! on early return would help traceability.