Skip to content

Conversation

@wangyum
Copy link
Member

@wangyum wangyum commented Jul 11, 2019

What changes were proposed in this pull request?

This pr enables spark.sql.crossJoin.enabled and spark.sql.parser.ansi.enabled for PostgreSQL test.

How was this patch tested?

manual tests:
Run test.sql in pgSQL directory and in inputs directory:

cat <<EOF > test.sql
create or replace temporary view t1 as
select * from (values(1), (2)) as v (val);

create or replace temporary view t2 as
select * from (values(2), (1)) as v (val);

select t1.*, t2.* from t1 join t2;
EOF

@wangyum
Copy link
Member Author

wangyum commented Jul 11, 2019

@maropu I didn't enable spark.sql.parser.ansi.enabled because:

spark-sql> set spark.sql.parser.ansi.enabled=true;
spark.sql.parser.ansi.enabled	true
spark-sql> select 1 as false;
Error in query:
no viable alternative at input 'false'(line 1, pos 12)

== SQL ==
select 1 as false
------------^^^

spark-sql> select 1 as minus;
Error in query:
no viable alternative at input 'minus'(line 1, pos 12)

== SQL ==
select 1 as minus
------------^^^

Is this we expected?

@maropu
Copy link
Member

maropu commented Jul 11, 2019

ur, looks bad.... I checkd the reserved keywords in Postgresql again;
https://www.postgresql.org/docs/current/sql-keywords-appendix.html
FALSE is reserved, but the qyery works well in postgresql;

postgres=# select 1 as false;
 false 
-------
     1
(1 row)

How about Oracle and the other databases? Can they accept that query, too?

@SparkQA
Copy link

SparkQA commented Jul 11, 2019

Test build #107509 has finished for PR 25109 at commit 86ca933.

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

@wangyum
Copy link
Member Author

wangyum commented Jul 11, 2019

Teradata, Vertica, Oracle, SQL Server and DB2 can accept these queries.

@wangyum
Copy link
Member Author

wangyum commented Jul 11, 2019

retest this please

@maropu
Copy link
Member

maropu commented Jul 11, 2019

oh, I see.... If so, I think Spark might need to accept these queries with ansi=true...

@maropu
Copy link
Member

maropu commented Jul 11, 2019

IMO we need to make spark compatible with these DBMSs with ansi=true. So, how about setting ansi=true for the PostgreSQL tests by default and turning off the ansi mode for these queries like this?

set ansi=false;
select 1 as false;
set ansi=true;

Then, filing a jira for these unsupported behaviours?

@SparkQA
Copy link

SparkQA commented Jul 11, 2019

Test build #107521 has finished for PR 25109 at commit 86ca933.

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

@SparkQA
Copy link

SparkQA commented Jul 13, 2019

Test build #107626 has finished for PR 25109 at commit 45af58f.

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

@wangyum wangyum changed the title [SPARK-28343][SQL][TEST] PostgreSQL test should enable cartesian product [SPARK-28343][SQL][TEST] PostgreSQL test enables cartesian product and ansi mode Jul 13, 2019
@wangyum wangyum changed the title [SPARK-28343][SQL][TEST] PostgreSQL test enables cartesian product and ansi mode [SPARK-28343][SQL][TEST] Enabling cartesian product and ansi mode for PostgreSQL testing Jul 13, 2019
@SparkQA
Copy link

SparkQA commented Jul 13, 2019

Test build #107628 has finished for PR 25109 at commit 4a66a89.

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


SELECT '' AS five, q1, q2, q1 + q2 AS plus FROM INT8_TBL;
SELECT '' AS five, q1, q2, q1 - q2 AS minus FROM INT8_TBL;
SELECT '' AS five, q1, q2, q1 - q2 AS `minus` FROM INT8_TBL;
Copy link
Member

Choose a reason for hiding this comment

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

Please add SPARK-28349 before this line.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jul 13, 2019

Thank you for swift update according to the decision, @wangyum !
If we add SPARK-28349 comment explicitly, the others looks good.
(If we forgot to add that comment, the future developers will ask about that.)

cc @gatorsmile and @maropu .

@SparkQA
Copy link

SparkQA commented Jul 14, 2019

Test build #107634 has finished for PR 25109 at commit 8ce7af4.

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

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, @wangyum and @maropu !

@wangyum wangyum deleted the SPARK-28343 branch July 14, 2019 06:45
vinodkc pushed a commit to vinodkc/spark that referenced this pull request Jul 18, 2019
… PostgreSQL testing

## What changes were proposed in this pull request?

This pr enables `spark.sql.crossJoin.enabled` and `spark.sql.parser.ansi.enabled` for PostgreSQL test.

## How was this patch tested?

manual tests:
Run `test.sql` in [pgSQL](https://github.com/apache/spark/tree/master/sql/core/src/test/resources/sql-tests/inputs/pgSQL) directory and in [inputs](https://github.com/apache/spark/tree/master/sql/core/src/test/resources/sql-tests/inputs) directory:
```sql
cat <<EOF > test.sql
create or replace temporary view t1 as
select * from (values(1), (2)) as v (val);

create or replace temporary view t2 as
select * from (values(2), (1)) as v (val);

select t1.*, t2.* from t1 join t2;
EOF
```

Closes apache#25109 from wangyum/SPARK-28343.

Authored-by: Yuming Wang <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
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.

4 participants