Skip to content

Conversation

@maryannxue
Copy link
Contributor

What changes were proposed in this pull request?

This PR fixes a bug where a CTE reference cannot be resolved if this reference occurs in an inner CTE definition nested in the outer CTE's main body FROM clause. E.g.,

WITH cte_outer AS (
  SELECT 1
)
SELECT * FROM (
  WITH cte_inner AS (
    SELECT * FROM cte_outer
  )
  SELECT * FROM cte_inner
)

This fix is to change the CTESubstitution's traverse order from resolveOperatorsUpWithPruning to resolveOperatorsDownWithPruning and also to recursively call traverseAndSubstituteCTE for CTE main body.

Why are the changes needed?

Bug fix. Without the fix an AnalysisException would be thrown for CTE queries mentioned above.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added UTs.

@github-actions github-actions bot added the SQL label Sep 1, 2022
@maryannxue
Copy link
Contributor Author

cc @cloud-fan

@cloud-fan cloud-fan closed this in f4ff2d1 Sep 1, 2022
@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan
Copy link
Contributor

It has conflicts in 3.3, due to missing #36146 . @peter-toth can you help to open a backport PR for it? Thanks!

@peter-toth
Copy link
Contributor

peter-toth commented Sep 1, 2022

It has conflicts in 3.3, due to missing #36146 . @peter-toth can you help to open a backport PR for it? Thanks!

Sure, I can open it today or tomorrow.

@peter-toth
Copy link
Contributor

@cloud-fan, here it is: #37760

cloud-fan pushed a commit that referenced this pull request Sep 6, 2022
… be resolved

This PR fixes a bug where a CTE reference cannot be resolved if this reference occurs in an inner CTE definition nested in the outer CTE's main body FROM clause. E.g.,
```
WITH cte_outer AS (
  SELECT 1
)
SELECT * FROM (
  WITH cte_inner AS (
    SELECT * FROM cte_outer
  )
  SELECT * FROM cte_inner
)
```

This fix is to change the `CTESubstitution`'s traverse order from `resolveOperatorsUpWithPruning` to `resolveOperatorsDownWithPruning` and also to recursively call `traverseAndSubstituteCTE` for CTE main body.

Bug fix. Without the fix an `AnalysisException` would be thrown for CTE queries mentioned above.

No.

Added UTs.

Closes #37751 from maryannxue/spark-40297.

Authored-by: Maryann Xue <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
@cloud-fan
Copy link
Contributor

backported to 3.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants