[SPARK-28390][SQL][PYTHON][TESTS] Convert and port 'pgSQL/select_having.sql' into UDF test base #25161

shivusondur · 2019-07-15T12:37:29Z

What changes were proposed in this pull request?

changed the test according to steps mentioned in SPARK-27921

difference comparing to select_having.sql

diff --git a/sql/core/src/test/resources/sql-tests/results/pgSQL/select_having.sql.out b/sql/core/src/test/resources/sql-tests/results/udf/pgSQL/udf-select_having.sql.out
index 02536eb..f731d11 100644
--- a/sql/core/src/test/resources/sql-tests/results/pgSQL/select_having.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/udf/pgSQL/udf-select_having.sql.out
@@ -91,54 +91,54 @@ struct<>
 
 
 -- !query 11
-SELECT b, c FROM test_having
-	GROUP BY b, c HAVING count(*) = 1 ORDER BY b, c
+SELECT udf(b), udf(c) FROM test_having
+	GROUP BY b, c HAVING udf(count(*)) = 1 ORDER BY udf(b), udf(c)
 -- !query 11 schema
-struct<b:int,c:string>
+struct<CAST(udf(cast(b as string)) AS INT):int,CAST(udf(cast(c as string)) AS STRING):string>
 -- !query 11 output
 1	XXXX
 3	bbbb
 
 
 -- !query 12
-SELECT b, c FROM test_having
-	GROUP BY b, c HAVING b = 3 ORDER BY b, c
+SELECT udf(b), udf(c) FROM test_having
+	GROUP BY b, c HAVING udf(b) = 3 ORDER BY udf(b), udf(c)
 -- !query 12 schema
-struct<b:int,c:string>
+struct<CAST(udf(cast(b as string)) AS INT):int,CAST(udf(cast(c as string)) AS STRING):string>
 -- !query 12 output
 3	BBBB
 3	bbbb
 
 
 -- !query 13
-SELECT c, max(a) FROM test_having
-	GROUP BY c HAVING count(*) > 2 OR min(a) = max(a)
+SELECT udf(c), max(udf(a)) FROM test_having
+	GROUP BY c HAVING udf(count(*)) > 2 OR udf(min(a)) = udf(max(a))
 	ORDER BY c
 -- !query 13 schema
-struct<c:string,max(a):int>
+struct<CAST(udf(cast(c as string)) AS STRING):string,max(CAST(udf(cast(a as string)) AS INT)):int>
 -- !query 13 output
 XXXX	0
 bbbb	5
 
 
 -- !query 14
-SELECT min(a), max(a) FROM test_having HAVING min(a) = max(a)
+SELECT udf(udf(min(udf(a)))), udf(udf(max(udf(a)))) FROM test_having HAVING udf(udf(min(udf(a)))) = udf(udf(max(udf(a))))
 -- !query 14 schema
-struct<min(a):int,max(a):int>
+struct<CAST(udf(cast(cast(udf(cast(min(cast(udf(cast(a as string)) as int)) as string)) as int) as string)) AS INT):int,CAST(udf(cast(cast(udf(cast(max(cast(udf(cast(a as string)) as int)) as string)) as int) as string)) AS INT):int>
 -- !query 14 output
 
 
 
 -- !query 15
-SELECT min(a), max(a) FROM test_having HAVING min(a) < max(a)
+SELECT udf(min(udf(a))), udf(udf(max(a))) FROM test_having HAVING udf(min(a)) < udf(max(udf(a)))
 -- !query 15 schema
-struct<min(a):int,max(a):int>
+struct<CAST(udf(cast(min(cast(udf(cast(a as string)) as int)) as string)) AS INT):int,CAST(udf(cast(cast(udf(cast(max(a) as string)) as int) as string)) AS INT):int>
 -- !query 15 output
 0	9
 
 
 -- !query 16
-SELECT a FROM test_having HAVING min(a) < max(a)
+SELECT udf(a) FROM test_having HAVING udf(min(a)) < udf(max(a))
 -- !query 16 schema
 struct<>
 -- !query 16 output
@@ -147,16 +147,16 @@ grouping expressions sequence is empty, and 'default.test_having.`a`' is not an
 
 
 -- !query 17
-SELECT 1 AS one FROM test_having HAVING a > 1
+SELECT 1 AS one FROM test_having HAVING udf(a) > 1
 -- !query 17 schema
 struct<>
 -- !query 17 output
 org.apache.spark.sql.AnalysisException
-cannot resolve '`a`' given input columns: [one]; line 1 pos 40
+cannot resolve '`a`' given input columns: [one]; line 1 pos 44
 
 
 -- !query 18
-SELECT 1 AS one FROM test_having HAVING 1 > 2
+SELECT 1 AS one FROM test_having HAVING udf(udf(1) > udf(2))
 -- !query 18 schema
 struct<one:int>
 -- !query 18 output
@@ -164,7 +164,7 @@ struct<one:int>
 
 
 -- !query 19
-SELECT 1 AS one FROM test_having HAVING 1 < 2
+SELECT 1 AS one FROM test_having HAVING udf(udf(1) < udf(2))
 -- !query 19 schema
 struct<one:int>
 -- !query 19 output
@@ -172,7 +172,7 @@ struct<one:int>
 
 
 -- !query 20
-SELECT 1 AS one FROM test_having WHERE 1/a = 1 HAVING 1 < 2
+SELECT 1 AS one FROM test_having WHERE 1/udf(a) = 1 HAVING 1 < 2
 -- !query 20 schema
 struct<one:int>
 -- !query 20 output

How was this patch tested?

by:

sudo SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/test-only *SQLQueryTestSuite -- -z udf/pgSQL/udf-select_having.sql"

HyukjinKwon · 2019-07-15T12:41:51Z

Thanks for working on this. Let's wait for #25130. We need to fix UDF testing base itself

HyukjinKwon · 2019-07-15T12:43:07Z

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql

After merging the PR I pointed out, let's try HAVING udf(1 > 2) combination as well.

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql

HyukjinKwon · 2019-07-15T12:43:32Z

Can you post the diff in the PR description as guided in the parent JIRA?

shivusondur · 2019-07-15T13:27:49Z

Can you post the diff in the PR description as guided in the parent JIRA?

Done

HyukjinKwon · 2019-07-15T15:17:19Z

PR description isn't exactly guided. It should be foldable with

Details

html tag.

shivusondur · 2019-07-15T17:18:31Z

PR description isn't exactly guided. It should be foldable with
@HyukjinKwon
Handled, Thanks for reminding me.

HyukjinKwon · 2019-07-18T04:06:14Z

ok to test

HyukjinKwon · 2019-07-18T04:06:29Z

#25130 is merged. Can you rebase and update it against the current master?

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql

HyukjinKwon · 2019-07-18T04:11:31Z

Looks fine otherwise if the tests pass

SparkQA · 2019-07-18T06:55:11Z

Test build #107821 has finished for PR 25161 at commit 6f44282.

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

changed the test according to steps mentnioned in SPARK-27921

SparkQA · 2019-07-19T07:05:02Z

Test build #107886 has finished for PR 25161 at commit b0a4fd1.

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

HyukjinKwon · 2019-07-19T07:26:19Z

retest this please

SparkQA · 2019-07-19T11:08:23Z

Test build #107889 has finished for PR 25161 at commit b0a4fd1.

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

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql

SparkQA · 2019-07-21T21:48:52Z

Test build #107969 has finished for PR 25161 at commit fc65bcf.

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

HyukjinKwon · 2019-07-22T02:35:10Z

@shivusondur, is the diff correct? Looks weird:

diff --git a/sql/core/src/test/resources/sql-tests/results/pgSQL/select_having.sql.out b/sql/core/src/test/resources/sql-tests/results/udf/pgSQL/udf-select_having.sql.out
index 02536eb..96ccd15 100644
--- a/sql/core/src/test/resources/sql-tests/results/pgSQL/select_having.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/udf/pgSQL/udf-select_having.sql.out
@@ -1,5 +1,5 @@
 -- Automatically generated by SQLQueryTestSuite
--- Number of queries: 22
+-- Number of queries: 42

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql

SparkQA · 2019-07-22T07:05:02Z

Test build #107979 has finished for PR 25161 at commit a1ba9a8.

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

…r required.

HyukjinKwon · 2019-07-22T07:35:18Z

Thanks for updating and addressing comments. Looks fine. Let me take a final look again before merging it.

SparkQA · 2019-07-22T11:35:29Z

Test build #107997 has finished for PR 25161 at commit ef39cdd.

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

HyukjinKwon · 2019-07-22T11:45:44Z

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql

+-- SELECT_HAVING
+-- https://github.com/postgres/postgres/blob/REL_12_BETA2/src/test/regress/sql/select_having.sql
+--
+-- This test file was converted from inputs/pgSQL/select_having.sql


Can we add this todo on the top:

-- TODO: We should add UDFs in GROUP BY clause when [SPARK-28445] is resolved.

and remove same lines below?

HyukjinKwon · 2019-07-22T11:47:08Z

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql

+-- Per SQL spec, these should generate 0 or 1 row, even without aggregates
+
+SELECT udf(udf(min(udf(a)))), udf(udf(max(udf(a)))) FROM test_having HAVING udf(udf(min(udf(a)))) = udf(udf(max(udf(a))));
+SELECT udf(udf(min(udf(a)))), udf(udf(max(udf(a)))) FROM test_having HAVING udf(udf(min(udf(a)))) < udf(udf(max(udf(a))));


Can we use different UDF combination comparing to the one right above?

updated like below

SELECT udf(udf(min(udf(a)))), udf(udf(max(udf(a)))) FROM test_having HAVING udf(udf(min(udf(a)))) = udf(udf(max(udf(a))));
SELECT udf(min(udf(a))), udf(udf(max(a))) FROM test_having HAVING udf(min(a)) < udf(max(udf(a)));

HyukjinKwon

Looks good otherwise

SparkQA · 2019-07-22T21:35:57Z

Test build #108021 has finished for PR 25161 at commit c4ad657.

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

SparkQA · 2019-07-23T07:05:02Z

Test build #108031 has finished for PR 25161 at commit 8795d66.

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

shivusondur · 2019-07-23T09:03:30Z

retest this please

HyukjinKwon · 2019-07-24T00:53:23Z

retest this please

SparkQA · 2019-07-24T05:29:01Z

Test build #108068 has finished for PR 25161 at commit 8795d66.

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

HyukjinKwon · 2019-07-24T05:43:07Z

Merged to master.

HyukjinKwon reviewed Jul 15, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Outdated Show resolved Hide resolved

HyukjinKwon changed the title ~~[SPARK-28390][SQL][Test] Convert and port 'pgSQL/select_having.sql' into UDF test base~~ [SPARK-28390][SQL][PYTHON][TESTS] Convert and port 'pgSQL/select_having.sql' into UDF test base Jul 15, 2019

dongjoon-hyun added PYSPARK SQL TESTS labels Jul 16, 2019

HyukjinKwon reviewed Jul 18, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Outdated Show resolved Hide resolved

HyukjinKwon reviewed Jul 18, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Outdated Show resolved Hide resolved

HyukjinKwon reviewed Jul 18, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Outdated Show resolved Hide resolved

HyukjinKwon reviewed Jul 18, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Outdated Show resolved Hide resolved

shivusondur added 3 commits July 19, 2019 10:46

for SPARK-28390

80f2894

changed the test according to steps mentnioned in SPARK-27921

Handled th review comments

624fad5

Handled the few more comments

b0a4fd1

shivusondur force-pushed the jira28390 branch from 6f44282 to b0a4fd1 Compare July 19, 2019 06:10

HyukjinKwon reviewed Jul 19, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Outdated Show resolved Hide resolved

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Outdated Show resolved Hide resolved

Handled the few more comments

fc65bcf

HyukjinKwon reviewed Jul 22, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/udf/pgSQL/udf-select_having.sql Show resolved Hide resolved

Just added the TODO comment

a1ba9a8

Removed the extra queries(made like original) and added udf() whereve…

ef39cdd

…r required.

HyukjinKwon reviewed Jul 22, 2019

View reviewed changes

HyukjinKwon approved these changes Jul 22, 2019

View reviewed changes

Added different combination of udf

c4ad657

Only "TODO" comment is moved up

8795d66

HyukjinKwon closed this in 167fa04 Jul 24, 2019

shivusondur mentioned this pull request Aug 18, 2019

[SPARK-28390][SQL][PYTHON][TESTS] [FOLLOW-UP] Update the TODO with actual blocking JIRA IDs #25415

Closed

[SPARK-28390][SQL][PYTHON][TESTS] Convert and port 'pgSQL/select_having.sql' into UDF test base #25161

[SPARK-28390][SQL][PYTHON][TESTS] Convert and port 'pgSQL/select_having.sql' into UDF test base #25161

Uh oh!

Conversation

shivusondur commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

HyukjinKwon commented Jul 15, 2019

Uh oh!

HyukjinKwon Jul 15, 2019

Choose a reason for hiding this comment

Uh oh!

shivusondur Jul 15, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HyukjinKwon commented Jul 15, 2019

Uh oh!

shivusondur commented Jul 15, 2019

Uh oh!

HyukjinKwon commented Jul 15, 2019

Uh oh!

shivusondur commented Jul 15, 2019

Uh oh!

HyukjinKwon commented Jul 18, 2019

Uh oh!

HyukjinKwon commented Jul 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HyukjinKwon commented Jul 18, 2019

Uh oh!

SparkQA commented Jul 18, 2019

Uh oh!

SparkQA commented Jul 19, 2019

Uh oh!

HyukjinKwon commented Jul 19, 2019

Uh oh!

SparkQA commented Jul 19, 2019

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Jul 21, 2019

Uh oh!

HyukjinKwon commented Jul 22, 2019

Uh oh!

Uh oh!

SparkQA commented Jul 22, 2019

Uh oh!

HyukjinKwon commented Jul 22, 2019

Uh oh!

SparkQA commented Jul 22, 2019

Uh oh!

HyukjinKwon Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

shivusondur Jul 23, 2019

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

shivusondur Jul 22, 2019

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 22, 2019

Uh oh!

SparkQA commented Jul 23, 2019

Uh oh!

shivusondur commented Jul 23, 2019

Uh oh!

HyukjinKwon commented Jul 24, 2019

Uh oh!

shivusondur commented Jul 15, 2019 •

edited

Loading