Skip to content

[SPARK-29120][SQL][TESTS] Port create_view.sql #26290

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.

Sign up for GitHub

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

Jump to bottom
Closed
maropu wants to merge 4 commits into from
Closed

[SPARK-29120][SQL][TESTS] Port create_view.sql #26290

maropu wants to merge 4 commits into from

Conversation

@maropu
Copy link
Member

@maropu maropu commented Oct 29, 2019

What changes were proposed in this pull request?

This PR ports create_view.sql from PostgreSQL regression tests https://github.com/postgres/postgres/blob/REL_12_STABLE/src/test/regress/sql/create_view.sql

The expected results can be found in the link: https://github.com/postgres/postgres/blob/REL_12_STABLE/src/test/regress/expected/create_view.out

Why are the changes needed?

To check behaviour differences between Spark and PostgreSQL

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass the Jenkins. And, Comparison with PgSQL results

@maropu
Copy link
Member Author

maropu commented Oct 29, 2019

Since I've not filed some issues in jira, I set WIP now.

maropu
maropu commented Oct 29, 2019
View reviewed changes
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/create_view.sql
CREATE VIEW v6_temp AS SELECT * FROM base_table WHERE id IN (SELECT id FROM temp_table);
CREATE VIEW v7_temp AS SELECT t1.id, t2.a FROM base_table t1, (SELECT * FROM temp_table) t2;
-- [SPARK-XXXXX] Not allowed to create a permanent view by referencing a temporary view in EXISTS
CREATE VIEW v8_temp AS SELECT * FROM base_table WHERE EXISTS (SELECT 1 FROM temp_table);
Copy link
Member Author

@maropu maropu Oct 29, 2019

Choose a reason for hiding this comment

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

This test should fail and throw an exception with a message Not allowed to create a permanent view v8_temp by referencing a temporary view temp_table, but it passes now.

Copy link
Member Author

@maropu maropu Oct 29, 2019

Choose a reason for hiding this comment

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

https://issues.apache.org/jira/browse/SPARK-29630

maropu
maropu commented Oct 29, 2019
View reviewed changes
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/create_view.sql
CREATE VIEW v7_temp AS SELECT t1.id, t2.a FROM base_table t1, (SELECT * FROM temp_table) t2;
-- [SPARK-XXXXX] Not allowed to create a permanent view by referencing a temporary view in EXISTS
CREATE VIEW v8_temp AS SELECT * FROM base_table WHERE EXISTS (SELECT 1 FROM temp_table);
CREATE VIEW v9_temp AS SELECT * FROM base_table WHERE NOT EXISTS (SELECT 1 FROM temp_table);
Copy link
Member Author

@maropu maropu Oct 29, 2019

Choose a reason for hiding this comment

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

ditto

@maropu maropu changed the title [WIP][SPARK-29120][SQL][TESTS] Port create_view.sql [SPARK-29120][SQL][TESTS] Port create_view.sql Oct 29, 2019
@SparkQA
Copy link

SparkQA commented Oct 29, 2019

Test build #112825 has finished for PR 26290 at commit 9339a07.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • -- SELECT count(*) FROM pg_class where relname = 'pubview'
  • -- SELECT count(*) FROM pg_class where relname LIKE 'mytempview'

@SparkQA
Copy link

SparkQA commented Oct 29, 2019

Test build #112829 has finished for PR 26290 at commit fce4819.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member Author

maropu commented Oct 29, 2019

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Oct 29, 2019

Test build #112835 has finished for PR 26290 at commit fce4819.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 29, 2019

Test build #112848 has finished for PR 26290 at commit bf3897d.

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

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Oct 29, 2019

Thanks for taking care of completeness, @maropu .

@maropu
Copy link
Member Author

maropu commented Oct 29, 2019

@dongjoon-hyun @HyukjinKwon @wangyum Anyone can check this?

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Oct 30, 2019

I'm also checking now, @maropu ~

dongjoon-hyun
dongjoon-hyun reviewed Oct 30, 2019
View reviewed changes
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/create_view.sql Outdated
SELECT * FROM viewtest;

-- should fail
-- Spark can accept the DDL query below
Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting! Do you now why PostgreSQL disallow this DDL query?

Copy link
Member Author

@maropu maropu Oct 30, 2019

Choose a reason for hiding this comment

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

Yea, I didn't know this behaviour, too, and it looks interesting to me. Just in case, I'll file jira for the behaivour. If I know something about this, I'll leave comments there.

Copy link
Member Author

@maropu maropu Oct 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

FYI: I asked a PgSQL committer (who I know) why; it seems there is no strong reason not to support these view definitions and they, the PgSQL community, could do in theory. But, no one tries to make PRs for that, so they don't support now.

Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 30, 2019

Choose a reason for hiding this comment

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

Haha. Got it. Thanks!

dongjoon-hyun
dongjoon-hyun approved these changes Oct 30, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM (with minor comments). Thank you, @maropu .

dongjoon-hyun
dongjoon-hyun approved these changes Oct 30, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Merged to master.
Thank you, @maropu and @HyukjinKwon .
This is a test only PR and I tested the generated result locally.

@SparkQA
Copy link

SparkQA commented Oct 30, 2019

Test build #112926 has finished for PR 26290 at commit a9c1597.

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

@maropu
Copy link
Member Author

maropu commented Oct 30, 2019

Thanks, @dongjoon-hyun , for your quick review and merge!

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

Reviewers

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees

No one assigned

Labels

SQL TESTS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maropu @SparkQA @dongjoon-hyun @HyukjinKwon