Skip to content

Commit

Permalink
sql (fix): Resolve UNNEST earlier to prevent cyclic references (#3312)
Browse files Browse the repository at this point in the history
## Case 1

```sql
SELECT id, t.name FROM A CROSS JOIN UNNEST (name) AS t (name)
```

This SQL is valid but fails to resolve with the following error:

```
wvlet.airframe.sql.SQLError: [SyntaxError] line 1:45 name is ambiguous:
- *name:string <- A.name
- *name:?
```

Because the output attribute of AliasedRelation (`t (name)`) is included
in the source attributes when resolve `UNNEST (name)` here:


https://github.com/wvlet/airframe/blob/49d31bad29eccd2459eb4c02593d371c1b55082a/airframe-sql/src/main/scala/wvlet/airframe/sql/analyzer/TypeResolver.scala#L319-L321

- `r` is CrossJoin
- `r.inputAttributes` is `left.outputAttributes +
right.outputAttributes`
- `right` is AliasedRelation(Unnest)

I think output attributes of AliasedRelation shouldn't be in the scope
when Unnest is resolved but I didn't come up with an easy fix for this
issue.

## Case 2

```sql
SELECT id, t.name FROM A CROSS JOIN UNNEST (A.name) AS t (name)
```

This SQL is also valid but fails to resolve with the following error:

```
Found unresolved expressions in:
[sql]
SELECT id, t.name FROM A CROSS JOIN UNNEST (A.name) AS t (name)
[plan]
[Project]: (id:long, name:string, name:string) => (id:long, name:?)
  - *id:long <- A.id
  - UnresolvedAttribute(t.name)
  [CrossJoin]: (id:long, name:string, name:string) => (id:long, name:string, name:string)
    - NaturalJoin
    [TableScan] default.A:  => (id:long, name:string)
    [AliasedRelation]:  => (name:string)
      - ResolvedIdentifier(Id(t))
      [Unnest]:  => (name:string)
        - *A.name:string <- A.name
```

Maybe this is because the resolved UNNEST column has a qualifier. This
case might not be UNNEST specific.
  • Loading branch information
takezoe authored Jan 8, 2024
1 parent 549d2f8 commit e76a7b1
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ object TypeResolver extends LogSupport {
resolveTableRef ::
resolveJoinUsing ::
resolveSubquery ::
resolveJoinUnnest ::
resolveRegularRelation ::
resolveColumns ::
resolveAggregationIndexes ::
Expand Down Expand Up @@ -231,6 +232,22 @@ object TypeResolver extends LogSupport {
}
}

object resolveJoinUnnest extends RewriteRule {
override def apply(context: AnalyzerContext): PlanRewriter = {
case j @ Join(_, _, a @ AliasedRelation(u: Unnest, _, _, _), _, _) =>
j.copy(right = a.copy(child = resolveUnnest(u, j, context)))
case j @ Join(_, a @ AliasedRelation(u: Unnest, _, _, _), _, _, _) =>
j.copy(left = a.copy(child = resolveUnnest(u, j, context)))
}

private def resolveUnnest(u: Unnest, j: Join, context: AnalyzerContext): Unnest = {
val resolvedAttributes = j.outputAttributes.filter(_.resolved)
u.transformUpExpressions { case x: Identifier =>
resolveExpression(context, x, resolvedAttributes)
}.asInstanceOf[Unnest]
}
}

object resolveJoinUsing extends RewriteRule {
def apply(context: AnalyzerContext): PlanRewriter = {
case j @ Join(joinType, left, right, u @ JoinUsing(joinKeys, _), _) =>
Expand Down Expand Up @@ -375,8 +392,8 @@ object TypeResolver extends LogSupport {
} else {
a.copy(expr = resolved)
}
case SingleColumn(a: Attribute, qualifier, _, _) if a.resolved =>
a
case SingleColumn(a: Attribute, qualifier, tableAlias, _) if a.resolved =>
a.withQualifier(qualifier).withTableAlias(tableAlias)
case m: MultiSourceColumn =>
var changed = false
val resolvedInputs = m.inputs.map {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,34 @@ class TypeResolverTest extends AirSpec with ResolverTestHelper {
ResolvedAttribute("value", DataType.LongType, Some("t"), None, None, None)
)
}

test("un3: resolve UNNEST array column without CROSS JOIN") {
val p = analyze("SELECT id, n FROM A, UNNEST (name) AS t (n)")
p.outputAttributes shouldMatch { case List(c1: Attribute, c2: Attribute) =>
c1.fullName shouldBe "id"
c2.fullName shouldBe "n"
}
}

test("un4: resolve UNNEST array column with the same output name") {
val p = analyze("SELECT id, t.name FROM A CROSS JOIN UNNEST (name) AS t (name)")
p.outputAttributes shouldMatch { case List(c1: Attribute, c2: Attribute) =>
c1.fullName shouldBe "id"
c1.sourceColumns.map(_.column) shouldBe Seq(a1)
c2.fullName shouldBe "t.name"
c2.sourceColumns.map(_.column) shouldBe Seq(a2)
}
}

test("un5: resolve UNNEST array column with qualifier") {
val p = analyze("SELECT A.id, t.name FROM A INNER JOIN B ON A.id = B.id CROSS JOIN UNNEST (A.name) AS t (name)")
p.outputAttributes shouldMatch { case List(c1: Attribute, c2: Attribute) =>
c1.fullName shouldBe "A.id"
c1.sourceColumns.map(_.column) shouldBe Seq(a1)
c2.fullName shouldBe "t.name"
c2.sourceColumns.map(_.column) shouldBe Seq(a2)
}
}
}

test("resolve select from values") {
Expand Down Expand Up @@ -1149,4 +1177,19 @@ class TypeResolverTest extends AirSpec with ResolverTestHelper {
}
}
}

test("Resolve identifier refers to column in AliasedRelation") {
// with column alias (id -> t.xid)
val p1 = analyze("select t.xid from (select id from A) as t(xid)")
p1.outputAttributes shouldMatch { case List(col: ResolvedAttribute) =>
col.fullName shouldBe "t.xid"
col.sourceColumn.map(_.column) shouldBe Some(a1)
}
// without column alias (id -> t.td)
val p2 = analyze("select t.id from (select id from A) as t(id)")
p2.outputAttributes shouldMatch { case List(col: ResolvedAttribute) =>
col.fullName shouldBe "t.id"
col.sourceColumn.map(_.column) shouldBe Some(a1)
}
}
}

0 comments on commit e76a7b1

Please sign in to comment.