Skip to content

L3 join-predicate disambiguation via alias-qualified schema columns (3b follow-up to #5) #7

Description

@zzylol

Context

PR #5 made L3 scalar expressions fully positional (L3Expr::Column(ColumnId)): filters, projections, sort keys, and join predicates all resolve column identity by position ("3a"). The remaining "3b" piece — disambiguating a join predicate when the same column name appears on both sides (the common JOIN ON a.k = b.k, and true self-joins t a JOIN t b) — is deferred.

Today such a predicate resolves both refs to the first match in the concatenated left ++ right schema. This is latent: nothing consumes the join predicate yet (L4/L5 unimplemented) and the join structure (Join node, both scans, concatenated schema, aggregates over joins) is correct.

Why it isn't trivial

  1. The unoptimized plan puts ON in filter. The SQL front end lowers DataFusion's unoptimized LogicalPlan (deliberately, to avoid projection/predicate-pushdown rewrites). There, JOIN … ON x = y lives in the join's filter, not its on clause — extracting on keys is an optimizer pass. So a "resolve the left key against the left schema and the right key against the right schema" trick has no structured on pairs to use.
  2. True self-joins need alias qualification. In metrics a JOIN metrics b ON a.x = b.x both sides scan metrics; even preserving the relation qualifier (a/b) can't map a ref to a side without tracking the alias onto the schema columns.

Options

  • (i) Use the optimized plan (into_optimized_plan) so on is populated → resolve each pair's left key against the left schema, right key against the right (+offset). Lower churn, but the optimized plan also sets TableScan.projection, pushes predicates, and may restructure aggregates — needs re-validation of all existing SQL lowering behavior.
  • (ii) Alias-qualified schema columns — recommended. Fully general (self-joins + non-equi filters), matches DataFusion's qualified-schema model. Cost: churn across every Column construction (code + tests).

Touch points for (ii)

  • crates/core/src/intent_algebra/schema.rs: Column { …, table: Option<String> }.
  • query_expr::ColumnRef::Named → carry an optional table qualifier.
  • column_resolution::resolve_column_ref: match on (table, name) when qualified.
  • Join schema concatenation: preserve each side's qualifiers.
  • SQL front end (crates/lower/src/sql/mod.rs): handle SubqueryAlias as a rename (rewrite the scan schema's column qualifiers to the alias); df_expr_to_l2 carries Column.relation.
  • Update Column constructions across the workspace (incl. tests).

Acceptance

  • metrics JOIN hosts ON metrics.service = hosts.service → join predicate Compare(Column(1), Eq, Column(4)) (distinct positions, not Column(1) = Column(1)).
  • t a JOIN t b ON a.k = b.k disambiguates a.k vs b.k.
  • Full PromQL + SQL suite stays green; clippy -D warnings + fmt clean.

Follow-up to #5 (feat/promql-l1-l3).

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions