Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-34421][SQL][3.0] Resolve temporary functions and views in views with CTEs #31592

Conversation

peter-toth
Copy link
Contributor

What changes were proposed in this pull request?

This PR:

  • Fixes a bug that doesn't report analysis error when it should:
    CREATE TEMPORARY VIEW temp_view AS SELECT 0;
    CREATE VIEW view_on_temp_view AS WITH cte AS (SELECT * FROM temp_view) SELECT * FROM cte
    
    by properly checking temporary objects in view definitions with CTEs.

Why are the changes needed?

To fix a bug.

Does this PR introduce any user-facing change?

Yes, analysis error is thrown as expected.

How was this patch tested?

Added new UT + existing ones.

…s with CTEs

This PR:
- Fixes a bug that prevents analysis of:
  ```
  CREATE TEMPORARY VIEW temp_view AS WITH cte AS (SELECT temp_func(0)) SELECT * FROM cte;
  SELECT * FROM temp_view
  ```
  by throwing:
  ```
  Undefined function: 'temp_func'. This function is neither a registered temporary function nor a permanent function registered in the database 'default'.
  ```
- and doesn't report analysis error when it should:
  ```
  CREATE TEMPORARY VIEW temp_view AS SELECT 0;
  CREATE VIEW view_on_temp_view AS WITH cte AS (SELECT * FROM temp_view) SELECT * FROM cte
  ```
  by properly collecting temporary objects from VIEW definitions with CTEs.

- Minor refactor to make the affected code more readable.

To fix a bug introduced with apache#30567

Yes, the query works again.

Added new UT + existing ones.

Closes apache#31550 from peter-toth/SPARK-34421-temp-functions-in-views-with-cte.

Authored-by: Peter Toth <peter.toth@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@SparkQA
Copy link

SparkQA commented Feb 19, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39848/

@SparkQA
Copy link

SparkQA commented Feb 19, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39848/

@@ -3650,6 +3650,84 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
}

test("SPARK-34421: Resolve temporary objects in temporary views with CTEs") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new SQL temp view feature is not in 3.0, and I think we can remove this test, because it should pass in 3.0 without this fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it passes as expected. All right, removed in 92df840

@SparkQA
Copy link

SparkQA commented Feb 19, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39852/

@SparkQA
Copy link

SparkQA commented Feb 19, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39852/

@SparkQA
Copy link

SparkQA commented Feb 19, 2021

Test build #135268 has finished for PR 31592 at commit 9630b72.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Feb 19, 2021

Test build #135271 has finished for PR 31592 at commit 92df840.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

thanks, merging to 3.0!

cloud-fan pushed a commit that referenced this pull request Feb 22, 2021
…s with CTEs

### What changes were proposed in this pull request?
This PR:
- Fixes a bug that doesn't report analysis error when it should:
  ```
  CREATE TEMPORARY VIEW temp_view AS SELECT 0;
  CREATE VIEW view_on_temp_view AS WITH cte AS (SELECT * FROM temp_view) SELECT * FROM cte
  ```
  by properly checking temporary objects in view definitions with CTEs.

### Why are the changes needed?
To fix a bug.

### Does this PR introduce _any_ user-facing change?
Yes, analysis error is thrown as expected.

### How was this patch tested?
Added new UT + existing ones.

Closes #31592 from peter-toth/SPARK-34421-temp-functions-in-views-with-cte-3.0.

Authored-by: Peter Toth <peter.toth@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan cloud-fan closed this Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants