[SPARK-31467][SQL][TEST] Refactor the sql tests to prevent TableAlreadyExistsException #28239

turboFei · 2020-04-17T08:21:56Z

What changes were proposed in this pull request?

If we add UT in hive/SQLQuerySuite or other sql test suites and use table named test.
We may meet TableAlreadyExistsException.

org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException: Table or view 'test' already exists in database 'default'

The reason is that, there is some tests that does not clean up the tables/views.
In this PR, I add withTempViews for these tests.

Why are the changes needed?

To fix the TableAlreadyExistsException issue when adding an UT, which uses table named test or others, in some sql test suites, such as hive/SQLQuerySuite.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existed UT.

turboFei · 2020-04-17T09:02:24Z

Just add withTempView for the tests that do not clean up used tables/views.
cc @HyukjinKwon

maropu · 2020-04-18T22:31:04Z

ok to test

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

maropu · 2020-04-18T22:44:49Z

I think (trivial though), the change looks ok. Did you check them only in SQLQuerySuite? I mean, have you checked if the logs of the other test suites have no TableAlreadyExistsException?

SparkQA · 2020-04-19T01:06:43Z

Test build #121460 has finished for PR 28239 at commit cdde32f.

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

turboFei · 2020-04-19T02:54:56Z

I think the change looks reasonable. Did you check them only in SQLQuerySuite? I mean, have you checked if the logs of the other test suites have no TableAlreadyExistsException?

Hi, I found this issue when adding UT into hive/SQLQuerySuite.
There is no TableAlreadyExistsException in the logs before add UT.

I just check the sql/SQLQuerySuite and refactor it too.

SparkQA · 2020-04-19T02:57:36Z

Test build #121467 has finished for PR 28239 at commit 565631b.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

turboFei · 2020-04-19T03:18:21Z

Will try to refactor other suites.

turboFei · 2020-04-19T04:42:41Z

I have used grep command to find all tests, which does clean up their used views, and refactor them.

cc @maropu

SparkQA · 2020-04-19T04:57:41Z

Test build #121466 has finished for PR 28239 at commit d699f58.

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

SparkQA · 2020-04-19T07:05:02Z

Test build #121468 has finished for PR 28239 at commit babffc7.

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

SparkQA · 2020-04-19T07:05:02Z

Test build #121470 has finished for PR 28239 at commit bb52763.

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

maropu · 2020-04-19T09:32:26Z

retest this please

maropu · 2020-04-19T09:36:53Z

I think createOrReplaceTempView doesn't throw TableAlreadyExistsException without withTempView. Does the title describe a different issue?

turboFei · 2020-04-19T09:39:11Z

Oh, I mean that if we createOrReplaceTempView and does not clean up it after this test.
If another test create a table with same name, TableAlreadyExistsException would be thrown.

maropu · 2020-04-19T09:42:16Z

Ur, I see. cc: @dongjoon-hyun

maropu · 2020-04-19T09:45:13Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

-        |AS (c STRING, d STRING)
-        |ROW FORMAT SERDE 'org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe'
-        |WITH SERDEPROPERTIES('field.delim' = '|')
+    withTempView("data") {


data -> test?

maropu · 2020-04-19T09:56:28Z

sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/json/JsonSuite.scala

+          """
+            |select complexArrayOfStruct[0].field1[1].inner2[0],
+            |complexArrayOfStruct[1].field2[0][1]
+            |from jsonTable
        """.stripMargin),


nit: wrong indents.

turboFei · 2020-04-19T10:19:22Z

thanks, will fix all indents related with """ """.

SparkQA · 2020-04-19T13:53:49Z

Test build #121473 has finished for PR 28239 at commit bb52763.

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

turboFei · 2020-04-19T14:57:19Z

Have refactor the indents and double check it.
cc @maropu @dongjoon-hyun

SparkQA · 2020-04-19T19:32:37Z

Test build #121477 has finished for PR 28239 at commit 453c5a5.

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

maropu · 2020-04-20T00:32:58Z

btw, since I think we tend to forget the cleanup, any other solution to automatically clean up temp views with low overheads after each test, if this issue's annoying?

turboFei · 2020-04-20T07:19:52Z

btw, since I think we tend to forget the cleanup, any other solution to automatically clean up temp views with low overheads after each test, if this issue's annoying?

It is difficult to identify which view should be cleaned up.
For example:

spark/sql/core/src/test/scala/org/apache/spark/sql/sources/TableScanSuite.scala

Lines 139 to 151 in 88d39e5

    
             override def beforeAll(): Unit = { 
        
               super.beforeAll() 
        
               sql( 
        
                 """ 
        
                   |CREATE TEMPORARY VIEW oneToTen 
        
                   |USING org.apache.spark.sql.sources.SimpleScanSource 
        
                   |OPTIONS ( 
        
                   |  From '1', 
        
                   |  To '10', 
        
                   |  option_with_underscores 'someval', 
        
                   |  option.with.dots 'someval' 
        
                   |) 
        
                 """.stripMargin)

This temp view is created in beforeAll, we should not clean up it after each test.

maropu · 2020-04-20T07:31:50Z

Ah, I see.

HyukjinKwon

Seems fine to me.

maropu · 2020-05-05T00:22:01Z

retest this please

maropu

LGTM, too.

SparkQA · 2020-05-05T04:59:18Z

Test build #122295 has finished for PR 28239 at commit 453c5a5.

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

…dyExistsException ### What changes were proposed in this pull request? If we add UT in hive/SQLQuerySuite or other sql test suites and use table named `test`. We may meet TableAlreadyExistsException. ``` org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException: Table or view 'test' already exists in database 'default' ``` The reason is that, there is some tests that does not clean up the tables/views. In this PR, I add `withTempViews` for these tests. ### Why are the changes needed? To fix the TableAlreadyExistsException issue when adding an UT, which uses table named `test` or others, in some sql test suites, such as hive/SQLQuerySuite. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Existed UT. Closes #28239 from turboFei/SPARK-31467. Authored-by: turbofei <fwang12@ebay.com> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org> (cherry picked from commit 8d1f7d2) Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>

maropu · 2020-05-05T06:15:18Z

Thanks! Merged to master/3.0.

SPARK-31467: Fix test issue with table named in hive/SQLQuerySuite

2a0719e

probot-autolabeler bot added the SQL label Apr 17, 2020

turboFei changed the title ~~SPARK-31467: Fix test issue with table named in hive/SQLQuerySuite~~ SPARK-31467: Fix issue when adding UT uses table named test in hive/SQLQuerySuite Apr 17, 2020

complete

cfb9aa8

turboFei changed the title ~~SPARK-31467: Fix issue when adding UT uses table named test in hive/SQLQuerySuite~~ SPARK-31467: Refactor the tests in hive/SQLQuerySuite to prevent TableAlreadyExistsException Apr 17, 2020

fix style

cdde32f

maropu changed the title ~~SPARK-31467: Refactor the tests in hive/SQLQuerySuite to prevent TableAlreadyExistsException~~ [SPARK-31467][SQL][TEST] Refactor the tests in hive/SQLQuerySuite to prevent TableAlreadyExistsException Apr 18, 2020

maropu reviewed Apr 18, 2020

View reviewed changes

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala Outdated Show resolved Hide resolved

maropu reviewed Apr 18, 2020

View reviewed changes

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala Outdated Show resolved Hide resolved

fix nit

d699f58

turboFei force-pushed the SPARK-31467 branch from 09c8c50 to d699f58 Compare April 19, 2020 02:30

refactor tests in sql/SQLQuerySuite

babffc7

turboFei changed the title ~~[SPARK-31467][SQL][TEST] Refactor the tests in hive/SQLQuerySuite to prevent TableAlreadyExistsException~~ [SPARK-31467][SQL][TEST] Refactor the sql tests to prevent TableAlreadyExistsException Apr 19, 2020

turboFei force-pushed the SPARK-31467 branch from 565631b to babffc7 Compare April 19, 2020 02:59

complete all relative suites

bb52763

probot-autolabeler bot added the STRUCTURED STREAMING label Apr 19, 2020

maropu reviewed Apr 19, 2020

View reviewed changes

fix indents error

2353c33

turboFei added 2 commits April 19, 2020 22:43

fix indents

f89c04a

fix indents

453c5a5

HyukjinKwon reviewed May 4, 2020

View reviewed changes

maropu approved these changes May 5, 2020

View reviewed changes

maropu closed this in 8d1f7d2 May 5, 2020

turboFei deleted the SPARK-31467 branch May 5, 2020 08:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-31467][SQL][TEST] Refactor the sql tests to prevent TableAlreadyExistsException #28239

[SPARK-31467][SQL][TEST] Refactor the sql tests to prevent TableAlreadyExistsException #28239

turboFei commented Apr 17, 2020 •

edited

turboFei commented Apr 17, 2020

maropu commented Apr 18, 2020

maropu commented Apr 18, 2020 •

edited

SparkQA commented Apr 19, 2020

turboFei commented Apr 19, 2020 •

edited

SparkQA commented Apr 19, 2020

turboFei commented Apr 19, 2020

turboFei commented Apr 19, 2020

SparkQA commented Apr 19, 2020

SparkQA commented Apr 19, 2020

SparkQA commented Apr 19, 2020

maropu commented Apr 19, 2020

maropu commented Apr 19, 2020

turboFei commented Apr 19, 2020

maropu commented Apr 19, 2020

maropu Apr 19, 2020 •

edited

maropu Apr 19, 2020

turboFei commented Apr 19, 2020 •

edited

SparkQA commented Apr 19, 2020

turboFei commented Apr 19, 2020

SparkQA commented Apr 19, 2020

maropu commented Apr 20, 2020 •

edited

turboFei commented Apr 20, 2020 •

edited

maropu commented Apr 20, 2020

HyukjinKwon left a comment

maropu commented May 5, 2020

maropu left a comment

SparkQA commented May 5, 2020

maropu commented May 5, 2020

[SPARK-31467][SQL][TEST] Refactor the sql tests to prevent TableAlreadyExistsException #28239

[SPARK-31467][SQL][TEST] Refactor the sql tests to prevent TableAlreadyExistsException #28239

Conversation

turboFei commented Apr 17, 2020 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

turboFei commented Apr 17, 2020

maropu commented Apr 18, 2020

maropu commented Apr 18, 2020 • edited

SparkQA commented Apr 19, 2020

turboFei commented Apr 19, 2020 • edited

SparkQA commented Apr 19, 2020

turboFei commented Apr 19, 2020

turboFei commented Apr 19, 2020

SparkQA commented Apr 19, 2020

SparkQA commented Apr 19, 2020

SparkQA commented Apr 19, 2020

maropu commented Apr 19, 2020

maropu commented Apr 19, 2020

turboFei commented Apr 19, 2020

maropu commented Apr 19, 2020

maropu Apr 19, 2020 • edited

Choose a reason for hiding this comment

maropu Apr 19, 2020

Choose a reason for hiding this comment

turboFei commented Apr 19, 2020 • edited

SparkQA commented Apr 19, 2020

turboFei commented Apr 19, 2020

SparkQA commented Apr 19, 2020

maropu commented Apr 20, 2020 • edited

turboFei commented Apr 20, 2020 • edited

maropu commented Apr 20, 2020

HyukjinKwon left a comment

Choose a reason for hiding this comment

maropu commented May 5, 2020

maropu left a comment

Choose a reason for hiding this comment

SparkQA commented May 5, 2020

maropu commented May 5, 2020

turboFei commented Apr 17, 2020 •

edited

maropu commented Apr 18, 2020 •

edited

turboFei commented Apr 19, 2020 •

edited

maropu Apr 19, 2020 •

edited

turboFei commented Apr 19, 2020 •

edited

maropu commented Apr 20, 2020 •

edited

turboFei commented Apr 20, 2020 •

edited