Skip to content

Commit dcee0f1

Browse files
h3n4lclaude
andcommitted
feat(trino/analysis): resolve view lineage via catalog metadata
A column selected through a view had lineage pointing at the view column. Masking configuration can only attach to table columns, so the consumer found no masker and returned the (possibly sensitive) value unmasked (BYT-9679). The consumer-side fix (bytebase#20560) is superseded by resolving views here, where it serves every consumer and unifies with the derived-relation resolver. Add GetQuerySpanWithCatalog: GetQuerySpan plus a *catalog.Catalog (the same type completion consumes; catalog.View gains a Definition field). A FROM reference the catalog resolves to a view binds as a DERIVED relation carrying its definition's resolved projection — computed recursively in the view's own catalog/schema context, cycle-guarded and memoized — so named references and stars through views, views over views, and definitions using CTEs/derived tables all reach the underlying base columns through the existing resolver machinery. The definition's relations join AccessTables (qualified with the view's context), and the view's metadata column names apply positionally over the definition's outputs (a count mismatch — stale metadata — makes the view opaque rather than risking a width-wrong expansion). A catalog-known base TABLE binds with its catalog columns: star expansion uses them (SELECT * over a base table or a mixed base+derived join expands to the exact projection), and relation column aliases over it (FROM customer AS c(i, p, …)) resolve to the renamed base columns. Resolution stays additive — the written ref is always retained — and a nil catalog leaves behaviour byte-identical to GetQuerySpan. Cross-reviewed (Codex): 4 findings — relation-column-alias lineage, stale- metadata count mismatch, and partial-projection memoization under cycles fixed; intermediate views in AccessTables kept intentionally (reading through a view accesses it). A focused re-verification pass confirmed the fixes sound. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent bbe8753 commit dcee0f1

5 files changed

Lines changed: 808 additions & 28 deletions

File tree

trino/analysis/query_span.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"strings"
55

66
"github.com/bytebase/omni/trino/ast"
7+
"github.com/bytebase/omni/trino/catalog"
78
"github.com/bytebase/omni/trino/parser"
89
)
910

@@ -102,6 +103,25 @@ type ColumnRef struct {
102103
// whatever parsed is still analyzed. On empty input it returns a zero-valued
103104
// span with Type=Unknown.
104105
func GetQuerySpan(statement string) (*QuerySpan, error) {
106+
return GetQuerySpanWithCatalog(statement, nil)
107+
}
108+
109+
// GetQuerySpanWithCatalog is GetQuerySpan with catalog metadata: a FROM
110+
// reference that resolves to a known VIEW is followed into its definition
111+
// (recursively, in the view's own catalog/schema context), so result-column
112+
// lineage reaches the underlying base-table columns and the definition's base
113+
// tables are added to AccessTables; a star over a catalog-known relation
114+
// expands to its exact projection. Unqualified names resolve against the
115+
// catalog's current session catalog/schema. A nil catalog (or names the
116+
// catalog cannot resolve) leaves behaviour identical to GetQuerySpan.
117+
func GetQuerySpanWithCatalog(statement string, cat *catalog.Catalog) (*QuerySpan, error) {
118+
return getQuerySpanWithViews(statement, newViewState(cat))
119+
}
120+
121+
// getQuerySpanWithViews is the shared implementation; vs carries the
122+
// catalog-aware resolution state across the recursive analysis of view
123+
// definitions (nil disables catalog resolution).
124+
func getQuerySpanWithViews(statement string, vs *viewState) (*QuerySpan, error) {
105125
file, _ := parser.Parse(statement)
106126
span := &QuerySpan{Type: Classify(statement)}
107127
if file == nil || len(file.Stmts) == 0 {
@@ -117,9 +137,18 @@ func GetQuerySpan(statement string) (*QuerySpan, error) {
117137
// FROM and CTE references): the primary walk records a derived column by the
118138
// name written at the reference site, which has no base table to mask
119139
// against. This rewrites those refs to the recovered base columns, leaving
120-
// direct base-table references untouched.
140+
// direct base-table references untouched. Base tables read through views
141+
// resolved during the pass are collected into this statement's
142+
// AccessTables.
121143
if len(file.Stmts) > 0 {
122-
resolveDerivedLineage(file.Stmts[0], span)
144+
var viewTables []TableAccess
145+
if vs != nil {
146+
savedOut := vs.tablesOut
147+
vs.tablesOut = &viewTables
148+
defer func() { vs.tablesOut = savedOut }()
149+
}
150+
resolveDerivedLineage(file.Stmts[0], span, vs)
151+
appendViewTables(span, viewTables)
123152
}
124153
return span, nil
125154
}

trino/analysis/query_span_resolve.go

Lines changed: 93 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,18 @@ import (
3737
// columns (width and order), so `SELECT *` over a derived table or CTE masks
3838
// positionally; a star that needs catalog metadata (base table, coalescing
3939
// join, unresolved relation) stays a single opaque "*" result, as before.
40-
func resolveDerivedLineage(stmt ast.Node, span *QuerySpan) {
40+
func resolveDerivedLineage(stmt ast.Node, span *QuerySpan, vs *viewState) {
4141
if span == nil {
4242
return
4343
}
4444
qs, ok := stmt.(*parser.QueryStmt)
4545
if !ok || qs.Query == nil {
4646
return
4747
}
48+
// The root CTE scope carries the catalog-aware resolution state (nil
49+
// disables catalog resolution); buildCTEDefs propagates it to nested
50+
// scopes.
51+
root := &cteDefs{views: vs}
4852
// Recompute the outermost query's column lineage with relation-scope
4953
// resolution (derived tables, CTEs, UNNEST, scalar subqueries, set-operation
5054
// arm merging, and star expansion) and fold it into the primary walk's
@@ -60,7 +64,7 @@ func resolveDerivedLineage(stmt ast.Node, span *QuerySpan) {
6064
// - an opaque star (not expandable without metadata) is left byte-for-byte
6165
// untouched, preserving the exact result shape consumers key on to apply
6266
// their own metadata-based expansion.
63-
cols := resolveQueryCols(qs.Query, nil)
67+
cols := resolveQueryCols(qs.Query, root)
6468

6569
// A top-level VALUES produces no Results in the primary walk (it has no
6670
// select items), leaving the consumer with zero positional maskers — a
@@ -164,10 +168,12 @@ func hasOpaque(cols []outCol) bool {
164168

165169
// cteDefs maps a CTE name (lower-cased) to its resolved output columns, with a
166170
// parent link for nested WITH scopes. A CTE shadows an outer one of the same
167-
// name; an inner WITH's definitions chain to the outer ones.
171+
// name; an inner WITH's definitions chain to the outer ones. The chain also
172+
// carries the catalog-aware resolution state (views) from the root scope.
168173
type cteDefs struct {
169174
defs map[string][]outCol
170175
parent *cteDefs
176+
views *viewState
171177
}
172178

173179
func (c *cteDefs) lookup(name string) ([]outCol, bool) {
@@ -180,6 +186,17 @@ func (c *cteDefs) lookup(name string) ([]outCol, bool) {
180186
return nil, false
181187
}
182188

189+
// viewState returns the catalog-aware resolution state carried by this scope
190+
// chain, or nil when there is none (catalog-less analysis).
191+
func (c *cteDefs) viewState() *viewState {
192+
for cur := c; cur != nil; cur = cur.parent {
193+
if cur.views != nil {
194+
return cur.views
195+
}
196+
}
197+
return nil
198+
}
199+
183200
// buildCTEDefs resolves a query's WITH clause into a cteDefs scope chained to
184201
// parent. Each CTE body is resolved in a scope that already includes the earlier
185202
// siblings (sequential visibility), matching standard non-recursive CTE scoping;
@@ -190,6 +207,9 @@ func buildCTEDefs(q *parser.Query, parent *cteDefs) *cteDefs {
190207
return parent
191208
}
192209
d := &cteDefs{defs: make(map[string][]outCol), parent: parent}
210+
if parent != nil {
211+
d.views = parent.views
212+
}
193213
for i := range q.With.CTEs {
194214
nq := q.With.CTEs[i]
195215
name := identName(nq.Name)
@@ -238,10 +258,32 @@ func resolveNodeCols(node parser.QueryNode, cte *cteDefs) []outCol {
238258
// TABLE name == SELECT * FROM name. Over an in-scope CTE the projection
239259
// is the CTE's resolved columns (verified against Trino 481:
240260
// `WITH w AS (SELECT phone, name …) TABLE w` returns [phone, name]);
241-
// over a base table the star needs catalog metadata and stays opaque.
242-
if parts := normalizedParts(n.Name); len(parts) == 1 && cte != nil {
243-
if cols, ok := cte.lookup(parts[0]); ok && len(cols) > 0 && !hasOpaque(cols) {
244-
return stampStar(cols, 0, nil)
261+
// the same holds for a catalog-resolved view (its definition's
262+
// projection) or table (its catalog columns). Otherwise the star needs
263+
// metadata the analysis does not have and stays opaque.
264+
parts := normalizedParts(n.Name)
265+
if len(parts) == 1 && cte != nil {
266+
if cols, ok := cte.lookup(parts[0]); ok {
267+
if len(cols) > 0 && !hasOpaque(cols) {
268+
return stampStar(cols, 0, nil)
269+
}
270+
return []outCol{{name: "*", opaque: true}}
271+
}
272+
}
273+
if vs := cte.viewState(); vs != nil {
274+
if key, view, table, found := vs.lookupRelation(parts); found {
275+
var cols []outCol
276+
if view != nil {
277+
if proj := vs.projectionFor(key, view); proj != nil {
278+
vs.collectTables(proj.tables)
279+
cols = proj.cols
280+
}
281+
} else {
282+
cols = tableCols(key, table)
283+
}
284+
if len(cols) > 0 && !hasOpaque(cols) {
285+
return stampStar(cols, 0, nil)
286+
}
245287
}
246288
}
247289
return []outCol{{name: "*", opaque: true}}
@@ -468,9 +510,11 @@ func (s *rscope) add(rel parser.Relation, alias string, colAliases []*ast.Identi
468510
case *parser.ParenRelation:
469511
s.add(n.Inner, alias, colAliases)
470512
case *parser.TableRelation:
513+
parts := normalizedParts(n.Name)
471514
// A single-part name that matches an in-scope CTE is a derived relation
472-
// (the CTE's resolved columns); otherwise it is a base table.
473-
if parts := normalizedParts(n.Name); len(parts) == 1 && s.cte != nil {
515+
// (the CTE's resolved columns); a CTE shadows any same-named catalog
516+
// object.
517+
if len(parts) == 1 && s.cte != nil {
474518
if cols, ok := s.cte.lookup(parts[0]); ok {
475519
name := alias
476520
if name == "" {
@@ -484,6 +528,29 @@ func (s *rscope) add(rel parser.Relation, alias string, colAliases []*ast.Identi
484528
if name == "" {
485529
name = lastPart(n.Name)
486530
}
531+
// Resolve against the catalog when one was supplied. A VIEW binds as a
532+
// derived relation carrying its definition's resolved projection, so
533+
// references and stars through it reach the underlying base columns; a
534+
// TABLE binds with its catalog columns, which star expansion uses (named
535+
// references through a base table keep their written form — additive
536+
// resolution needs no help there, see resolveRef).
537+
if vs := s.cte.viewState(); vs != nil {
538+
if key, view, table, found := vs.lookupRelation(parts); found {
539+
if view != nil {
540+
if proj := vs.projectionFor(key, view); proj != nil {
541+
vs.collectTables(proj.tables)
542+
s.rels = append(s.rels, rbind{name: name, derived: true, cols: applyColumnAliases(proj.cols, colAliases)})
543+
return
544+
}
545+
// Unresolvable view (no/unanalyzable definition, cycle):
546+
// bind opaquely so stars through it stay unexpanded.
547+
s.rels = append(s.rels, rbind{name: name, derived: false})
548+
return
549+
}
550+
s.rels = append(s.rels, rbind{name: name, derived: false, cols: applyColumnAliases(tableCols(key, table), colAliases)})
551+
return
552+
}
553+
}
487554
s.rels = append(s.rels, rbind{name: name, derived: false})
488555
case *parser.SubqueryRelation:
489556
cols := resolveQueryCols(n.Query, s.cte)
@@ -542,10 +609,11 @@ func (s *rscope) unnestColumns(n *parser.UnnestRelation, colAliases []*ast.Ident
542609
// positional masker downstream, which is precisely the bug this resolves — so
543610
// it bails (nil) unless:
544611
// - no USING/NATURAL join coalesces columns in this scope, and
545-
// - every covered relation is a derived relation (subquery, CTE reference, or
546-
// aliased UNNEST) whose projection is fully resolved: non-empty and free of
547-
// opaque star placeholders. A base table (width known only to catalog
548-
// metadata), a lateral/table-function relation, an UNNEST without column
612+
// - every covered relation has a fully-resolved projection: non-empty and
613+
// free of opaque star placeholders. That is a derived relation (subquery,
614+
// CTE reference, aliased UNNEST, catalog-resolved view) or a base table
615+
// whose columns the supplied catalog knows. A base table without catalog
616+
// metadata, a lateral/table-function relation, an UNNEST without column
549617
// aliases, or a qualifier matching zero or several relations all bail.
550618
//
551619
// A nil return leaves the star opaque — the consumer's metadata-based expansion
@@ -563,14 +631,14 @@ func (s *rscope) starExpansion(qualifier string) []outCol {
563631
count++
564632
}
565633
}
566-
if count != 1 || !match.derived || len(match.cols) == 0 || hasOpaque(match.cols) {
634+
if count != 1 || len(match.cols) == 0 || hasOpaque(match.cols) {
567635
return nil
568636
}
569637
return match.cols
570638
}
571639
var out []outCol
572640
for _, rb := range s.rels {
573-
if !rb.derived || len(rb.cols) == 0 || hasOpaque(rb.cols) {
641+
if len(rb.cols) == 0 || hasOpaque(rb.cols) {
574642
return nil
575643
}
576644
out = append(out, rb.cols...)
@@ -617,24 +685,25 @@ func (s *rscope) resolveRef(ref ColumnRef) []ColumnRef {
617685
out := []ColumnRef{ref}
618686
if ref.Table != "" {
619687
// Qualified by a relation name: append the recovered sources of every
620-
// in-scope derived relation of that name exposing the column (a reused
621-
// alias yields several; unioning them over-includes, which is safe).
688+
// in-scope relation of that name exposing the column (a reused alias
689+
// yields several; unioning them over-includes, which is safe). A
690+
// catalog-known base table's cols matter when relation column aliases
691+
// rename its columns (FROM customer AS c(i, p, …): c.p must reach
692+
// customer.phone); without aliases they only restate the written ref
693+
// in qualified form, which is harmless.
622694
for _, rb := range s.rels {
623-
if rb.derived && strings.EqualFold(rb.name, ref.Table) {
695+
if strings.EqualFold(rb.name, ref.Table) {
624696
if src, ok := lookupOutCol(rb.cols, ref.Column); ok {
625697
out = append(out, src...)
626698
}
627699
}
628700
}
629701
return out
630702
}
631-
// Bare reference: append the recovered sources of every in-scope derived
632-
// relation exposing a column of this name. The original bare ref is retained
633-
// so a base table providing the same column is still covered.
703+
// Bare reference: append the recovered sources of every in-scope relation
704+
// exposing a column of this name. The original bare ref is retained so a
705+
// base table providing the same column is still covered.
634706
for _, rb := range s.rels {
635-
if !rb.derived {
636-
continue
637-
}
638707
if src, ok := lookupOutCol(rb.cols, ref.Column); ok {
639708
out = append(out, src...)
640709
}

0 commit comments

Comments
 (0)