From fd0ad2e3361d623786e9746814a369e83006c056 Mon Sep 17 00:00:00 2001 From: funcpp Date: Mon, 21 Sep 2026 14:56:19 +0900 Subject: [PATCH] fix: stop claiming a table for columns that resolve to none `ColumnOrigin::Concrete { table, column }` is a claim that the output column really derives from `table.column`. The resolver emitted that claim in two places where it had proven nothing, putting an invented name in the table slot: SELECT bare_col bare_col <- Concrete { table: "?unknown?", column: "bare_col" } WITH cte AS (SELECT present FROM source) SELECT missing FROM cte missing <- Concrete { table: "?cte?", column: "missing" } Neither name can appear in `tables.inputs`, so the column graph referenced relations the table graph denied existed, and a consumer had no way to separate proven lineage from a guess short of matching those sentinel strings. A third site already reported the same state differently: when every binding is a CTE or derived table and none has the column, no physical relation is left to attribute it to, and the result was `Ambiguous` with an empty candidate list. All three now produce `ColumnOrigin::Unresolved { column }`. That leaves each remaining variant with one meaning: `Concrete` is proven, `Ambiguous` is a real choice between at least two known relations, `Unresolved` is neither. Both invariants are documented. `apply_catalog` needs no change. It refines `Ambiguous`, and `Unresolved` is not `Ambiguous`, so a provider that resolves a column by name alone cannot turn an unresolved origin back into a fabricated concrete one -- the bug is unrepresentable rather than guarded against. `ColumnOrigin` is deliberately left exhaustive. It encodes how much was proven; a consumer that silently ignores a new resolution state is the failure it exists to prevent, so a new variant should break their build. This matches the rule ARCHITECTURE.md already states for sqlparser AST variants, and is why the CLI and the PyO3 bridge both had to be updated here rather than falling through a wildcard arm. Known limitation, pinned by a test: a column hidden behind an unexpanded `SELECT *` inside a CTE or derived table also reads as `Unresolved`, although the relation is known and the column very likely exists. Naming it needs an origin that can carry a relation. Co-Authored-By: Claude Opus 5 (1M context) --- ARCHITECTURE.md | 9 +++++ sqllineage-python/sqllineage.pyi | 8 +++- sqllineage-python/src/lib.rs | 11 ++++++ sqllineage/src/bin/sqllineage.rs | 1 + sqllineage/src/resolve/mod.rs | 17 +++++--- sqllineage/src/types.rs | 29 +++++++++++++- sqllineage/tests/catalog.rs | 62 ++++++++++++++++++++++++++++++ sqllineage/tests/column_lineage.rs | 34 +++++++++++++++- sqllineage/tests/cte.rs | 59 ++++++++++++++++++++++++++++ 9 files changed, 221 insertions(+), 9 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index ea445ec..805911b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -55,6 +55,15 @@ table recursively. Star nodes go through the same chain via `expand_star()`. Every resolve path handles all three binding types (Table, Cte, DerivedTable) uniformly. +A column that resolves to no relation at all becomes `Unresolved` rather than +a `Concrete` origin naming an invented table. `Concrete` is a claim that the +column really comes from that table, so the resolver only emits it once it has +one. `Ambiguous` is reserved for a genuine choice between two or more known +relations, which is the only case a `CatalogProvider` is asked to settle. +`ColumnOrigin` is deliberately not `#[non_exhaustive]`: a consumer that starts +silently ignoring a new resolution state is the failure the enum exists to +prevent, so a new variant should break their build. + `resolve/topo.rs` validates that the graph is a DAG after removing recursive CTE back-edges. `resolve/catalog.rs` applies the optional `CatalogProvider` as a post-processing step. diff --git a/sqllineage-python/sqllineage.pyi b/sqllineage-python/sqllineage.pyi index c6ccfc9..2f51219 100644 --- a/sqllineage-python/sqllineage.pyi +++ b/sqllineage-python/sqllineage.pyi @@ -16,8 +16,12 @@ class ColumnOrigin: """Resolution state of a source column. Check ``kind`` to determine the variant: - - ``"concrete"``: ``table`` and ``column`` are set. - - ``"ambiguous"``: ``column`` and ``candidates`` are set. + - ``"concrete"``: ``table`` and ``column`` are set. The only kind that + asserts the column really comes from that table. + - ``"ambiguous"``: ``column`` and ``candidates`` are set. ``candidates`` + always holds at least two tables. + - ``"unresolved"``: ``column`` is set. The column could not be traced to + any relation in scope, so there is no table to name. - ``"wildcard"``: ``table`` is set. - ``"recursive"``: ``base_sources`` is set. """ diff --git a/sqllineage-python/src/lib.rs b/sqllineage-python/src/lib.rs index 01e3128..8495054 100644 --- a/sqllineage-python/src/lib.rs +++ b/sqllineage-python/src/lib.rs @@ -115,6 +115,10 @@ impl PyColumnOrigin { "ColumnOrigin.ambiguous({})", self.column.as_deref().unwrap_or("?"), ), + "unresolved" => format!( + "ColumnOrigin.unresolved({})", + self.column.as_deref().unwrap_or("?"), + ), "recursive" => "ColumnOrigin.recursive(...)".to_string(), other => format!("ColumnOrigin.{other}(...)"), } @@ -137,6 +141,13 @@ fn convert_origin(o: &sqllineage_core::ColumnOrigin) -> PyColumnOrigin { candidates: Some(candidates.iter().map(PyTableRef::from).collect()), base_sources: None, }, + sqllineage_core::ColumnOrigin::Unresolved { column } => PyColumnOrigin { + kind: "unresolved".into(), + table: None, + column: Some(column.clone()), + candidates: None, + base_sources: None, + }, sqllineage_core::ColumnOrigin::Wildcard { table } => PyColumnOrigin { kind: "wildcard".into(), table: Some(PyTableRef::from(table)), diff --git a/sqllineage/src/bin/sqllineage.rs b/sqllineage/src/bin/sqllineage.rs index 112d450..8840db6 100644 --- a/sqllineage/src/bin/sqllineage.rs +++ b/sqllineage/src/bin/sqllineage.rs @@ -140,6 +140,7 @@ fn format_origin(origin: &ColumnOrigin) -> String { match origin { ColumnOrigin::Concrete { table, column } => format!("{table}.{column}"), ColumnOrigin::Ambiguous { column, .. } => format!("?{column}?"), + ColumnOrigin::Unresolved { column } => format!(""), ColumnOrigin::Wildcard { table } => format!("{table}.*"), ColumnOrigin::Recursive { base_sources } => { let inner: Vec = base_sources.iter().map(format_origin).collect(); diff --git a/sqllineage/src/resolve/mod.rs b/sqllineage/src/resolve/mod.rs index 8b7e732..9e2a121 100644 --- a/sqllineage/src/resolve/mod.rs +++ b/sqllineage/src/resolve/mod.rs @@ -433,8 +433,8 @@ fn resolve_from_bindings( } } } else if bindings.is_empty() { - Some(ColumnOrigin::Concrete { - table: TableRef::new("?unknown?"), + // No relation is visible at all, so nothing can own this column. + Some(ColumnOrigin::Unresolved { column: name.to_string(), }) } else { @@ -454,7 +454,13 @@ fn resolve_from_bindings( Binding::Table(t) => table_candidates.push(t.clone()), } } - if table_candidates.len() == 1 { + // Every binding was a CTE or derived table without this column, and no + // physical relation was left to attribute it to. + if table_candidates.is_empty() { + Some(ColumnOrigin::Unresolved { + column: name.to_string(), + }) + } else if table_candidates.len() == 1 { Some(ColumnOrigin::Concrete { table: table_candidates.into_iter().next().unwrap(), column: name.to_string(), @@ -497,8 +503,9 @@ fn resolve_through_scope( origins.into_iter().next() } } else { - Some(ColumnOrigin::Concrete { - table: TableRef::new("?cte?"), + // The scope is known but publishes no column of this name — either it + // genuinely has none, or an unexpanded `SELECT *` hides it. + Some(ColumnOrigin::Unresolved { column: column_name.to_string(), }) } diff --git a/sqllineage/src/types.rs b/sqllineage/src/types.rs index 94d11f7..b737ccc 100644 --- a/sqllineage/src/types.rs +++ b/sqllineage/src/types.rs @@ -118,15 +118,36 @@ pub struct ColumnMapping { } /// Resolution state of a source column. +/// +/// The variants record how much was actually proven. Match exhaustively: a +/// consumer that treats an unproven origin as proven is the failure this +/// distinction exists to prevent. #[derive(Debug, Clone, Serialize)] pub enum ColumnOrigin { /// Fully resolved to a specific table and column. Concrete { table: TableRef, column: String }, - /// Multiple candidate tables; catalog needed to disambiguate. + /// The column belongs to one of `candidates`, which relation is not + /// determined. A catalog can disambiguate it. + /// + /// `candidates` always holds at least two tables. A column that resolved + /// to exactly one table is [`Concrete`]; one that resolved to none is + /// [`Unresolved`]. + /// + /// [`Concrete`]: ColumnOrigin::Concrete + /// [`Unresolved`]: ColumnOrigin::Unresolved Ambiguous { column: String, candidates: Vec, }, + /// The column could not be traced to any relation in scope. + /// + /// This is not ambiguity between known tables — there is no candidate to + /// choose from, and a catalog is not consulted. It arises when no relation + /// is visible at all (`SELECT bare_col`), or when the CTE or derived table + /// the column was selected from has no output column of that name, which + /// happens both for a genuinely absent column and for one hidden behind an + /// unexpanded `SELECT *`. + Unresolved { column: String }, /// `SELECT *` or `table.*`; catalog needed to expand. Wildcard { table: TableRef }, /// Derived via recursive CTE; base case sources only. @@ -358,5 +379,11 @@ pub trait CatalogProvider { /// Return the column names of a table. Used to expand `SELECT *`. fn list_columns(&self, table: &TableRef) -> Option>; /// Given a column name and candidate tables, return the owning table. + /// + /// `candidates` always holds at least two tables — the relations the + /// column could have come from. Columns that resolved to no relation at + /// all are [`ColumnOrigin::Unresolved`] and are never passed here, because + /// a name existing somewhere in the catalog is not evidence that its table + /// takes part in this query. fn resolve_column(&self, column: &str, candidates: &[TableRef]) -> Option; } diff --git a/sqllineage/tests/catalog.rs b/sqllineage/tests/catalog.rs index 73bf2a6..4ab4635 100644 --- a/sqllineage/tests/catalog.rs +++ b/sqllineage/tests/catalog.rs @@ -128,3 +128,65 @@ fn catalog_preserves_qualified_columns() { vec![("orders".into(), "amount".into())] ); } + +/// Answers `resolve_column` for any name, the way a catalog that looks columns +/// up globally would. +struct ByNameCatalog; + +impl CatalogProvider for ByNameCatalog { + fn list_columns(&self, _table: &TableRef) -> Option> { + None + } + + fn resolve_column(&self, _column: &str, _candidates: &[TableRef]) -> Option { + Some(TableRef::new("guessed")) + } +} + +/// An unresolved column has no candidate relations, so there is nothing for a +/// catalog to choose between. A catalog that answers by name alone must not be +/// able to turn it into a concrete origin: a column existing somewhere in the +/// catalog is not evidence that its table takes part in this query. +#[test] +fn catalog_cannot_give_an_unresolved_column_an_owner() { + for sql in [ + "SELECT bare_col", + "WITH cte AS (SELECT present FROM source) SELECT bare_col FROM cte", + ] { + let result = analyze( + sql, + AnalyzeOptions { + catalog: Some(Box::new(ByNameCatalog)), + ..AnalyzeOptions::default() + }, + ) + .expect("parse") + .remove(0); + + let m = find_mapping(&result.columns.mappings, "bare_col"); + assert!( + matches!(&m.sources[..], [ColumnOrigin::Unresolved { .. }]), + "{sql}: got {:?}", + m.sources + ); + } +} + +/// The other side of the same boundary: genuine ambiguity between known tables +/// is still the catalog's to resolve. +#[test] +fn catalog_still_resolves_genuine_ambiguity() { + let result = analyze( + "SELECT name FROM users JOIN orders ON users.id = orders.user_id", + opts_with_catalog(), + ) + .expect("parse") + .remove(0); + + let m = find_mapping(&result.columns.mappings, "name"); + assert!( + matches!(&m.sources[..], [ColumnOrigin::Concrete { table, .. }] if table.table == "users"), + "got {:?}", + m.sources + ); +} diff --git a/sqllineage/tests/column_lineage.rs b/sqllineage/tests/column_lineage.rs index c906f68..4062579 100644 --- a/sqllineage/tests/column_lineage.rs +++ b/sqllineage/tests/column_lineage.rs @@ -1,7 +1,7 @@ mod common; use common::{analyze_one, concrete_sources, find_mapping, table}; -use sqllineage::TransformKind; +use sqllineage::{ColumnOrigin, TransformKind}; #[test] fn select_columns() { @@ -121,3 +121,35 @@ fn select_cast_passthrough() { assert_eq!(concrete_sources(m), vec![("t".into(), "a".into())]); assert_eq!(m.transform, TransformKind::Direct); } + +/// With no relation in scope there is nothing that could own the column, so +/// the origin says so instead of naming a table. +#[test] +fn bare_column_without_any_relation_is_unresolved() { + let result = analyze_one("SELECT bare_col"); + let m = find_mapping(&result.columns.mappings, "bare_col"); + assert!( + matches!(&m.sources[..], [ColumnOrigin::Unresolved { column }] if column == "bare_col"), + "got {:?}", + m.sources + ); +} + +/// A set operation can mix a proven origin with an unresolved one in the same +/// mapping, so completeness is a per-source question. +#[test] +fn proven_and_unresolved_sources_coexist_in_one_mapping() { + let result = analyze_one("SELECT a FROM t UNION ALL SELECT bare"); + let m = find_mapping(&result.columns.mappings, "a"); + assert!( + matches!( + &m.sources[..], + [ + ColumnOrigin::Concrete { table, .. }, + ColumnOrigin::Unresolved { column }, + ] if table.table == "t" && column == "bare" + ), + "got {:?}", + m.sources + ); +} diff --git a/sqllineage/tests/cte.rs b/sqllineage/tests/cte.rs index 24bdfbd..aedfc8a 100644 --- a/sqllineage/tests/cte.rs +++ b/sqllineage/tests/cte.rs @@ -255,3 +255,62 @@ fn cte_chain_select_star() { let m = find_mapping(&result.columns.mappings, "x"); assert_eq!(concrete_sources(m), vec![("t".into(), "x".into())]); } + +fn only_source(sql: &str, column: &str) -> ColumnOrigin { + let result = analyze_one(sql); + let m = find_mapping(&result.columns.mappings, column); + assert_eq!( + m.sources.len(), + 1, + "expected one source, got {:?}", + m.sources + ); + m.sources[0].clone() +} + +/// A CTE that has no output column of this name cannot be said to provide it. +/// Three shapes reach the same state, and all three used to claim a fabricated +/// `?cte?` table. +#[test] +fn column_absent_from_a_cte_is_unresolved() { + // The CTE genuinely has no such column. + let missing = only_source( + "WITH cte AS (SELECT present FROM source) SELECT missing FROM cte", + "missing", + ); + assert!( + matches!(&missing, ColumnOrigin::Unresolved { column } if column == "missing"), + "got {missing:?}" + ); + + // Every binding is a CTE and none of them has the column, so no physical + // relation is left to attribute it to. + let two_ctes = only_source( + "WITH a AS (SELECT p FROM s1), b AS (SELECT q FROM s2) \ + SELECT missing FROM a JOIN b ON a.p = b.q", + "missing", + ); + assert!( + matches!(&two_ctes, ColumnOrigin::Unresolved { column } if column == "missing"), + "got {two_ctes:?}" + ); +} + +/// A column hidden behind an unexpanded `SELECT *` reads as unresolved too. +/// +/// Here `p` very likely exists — the star just was not expanded. Reporting it +/// as unresolved is honest but coarse; naming the relation it came from needs +/// an origin that can carry one. Pinned so that change is visible when it lands. +#[test] +fn column_behind_an_unexpanded_star_is_unresolved() { + for sql in [ + "WITH cte AS (SELECT * FROM t) SELECT p FROM cte", + "SELECT p FROM (SELECT * FROM t) d", + ] { + let origin = only_source(sql, "p"); + assert!( + matches!(&origin, ColumnOrigin::Unresolved { column } if column == "p"), + "{sql}: got {origin:?}" + ); + } +}