You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/09/11 10:27:38 UTC
[GitHub] [spark] wangyum opened a new pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
wangyum opened a new pull request #29728:
URL: https://github.com/apache/spark/pull/29728
### What changes were proposed in this pull request?
This pr prohibit binary comparisons chain.
### Why are the changes needed?
1. There are potential issues in enabling the binary comparison chain. Wrong SQL written by users will affect the stability of the entire thrift server because of Spark sql will use `BroadcastNestedLoopJoin `. This is a real case:
2. Most SQL engine do not support
```
postgres=# create table t3(a int, b int, c int);
CREATE TABLE
postgres=# select * from t3 where a = b =c;
ERROR: syntax error at or near "="
```
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016872
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691374096
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016389
**[Test build #128566 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128566/testReport)** for PR 29728 at commit [`f8119e9`](https://github.com/apache/spark/commit/f8119e9cfcc9c5bd1a9f52852a89b73fdb394f56).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691417356
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016872
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691178532
**[Test build #128566 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128566/testReport)** for PR 29728 at commit [`f8119e9`](https://github.com/apache/spark/commit/f8119e9cfcc9c5bd1a9f52852a89b73fdb394f56).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691898796
> Most SQL engine do not support
>> postgres=# create table t3(a int, b int, c int);
>> CREATE TABLE
>> postgres=# select * from t3 where a = b =c;
>> ERROR: syntax error at or near "="
Is the example in the PR description PostgreSQL one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016389
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016872
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487147769
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016389
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487921098
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,29 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val isUnsupportedComparison = (left, right) match {
Review comment:
Total nit, but isn't it clearer to write `isSupportedComparison` and avoid inverting the logic everywhere?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691369915
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487147769
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691373868
**[Test build #128587 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128587/testReport)** for PR 29728 at commit [`92f1272`](https://github.com/apache/spark/commit/92f1272d16383046fdb260ca4fcafcbda104a6e0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691417115
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016872
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691180256
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-692491090
@srowen Standard SQL does not have this definition. In fact, `a = b = c` is the same as `a = b = cast(c as boolean)`. examples:
```sql
spark-sql> select 0 = 0 = 0;
false
spark-sql> select 1 = 1 = 1;
true
spark-sql> select 1 > 2 = 0;
true
spark-sql> select 2 < 1 = 'false';
true
spark-sql> select 2 < 1 = '2' = 'x';
NULL
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691373868
**[Test build #128587 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128587/testReport)** for PR 29728 at commit [`92f1272`](https://github.com/apache/spark/commit/92f1272d16383046fdb260ca4fcafcbda104a6e0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016872
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691374096
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016872
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016389
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691369737
**[Test build #128586 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128586/testReport)** for PR 29728 at commit [`1325c32`](https://github.com/apache/spark/commit/1325c32941139177e6b069e117977dcd0dd30cab).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691180256
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691369915
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016872
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487345912
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691417115
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487345912
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487147769
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,26 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val hasBinaryComparisons = left.isInstanceOf[BinaryComparison] ||
Review comment:
nit: hasBinaryComparisons => hasBinaryComparisonChildren
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016389
**[Test build #128566 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128566/testReport)** for PR 29728 at commit [`f8119e9`](https://github.com/apache/spark/commit/f8119e9cfcc9c5bd1a9f52852a89b73fdb394f56).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691416770
**[Test build #128586 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128586/testReport)** for PR 29728 at commit [`1325c32`](https://github.com/apache/spark/commit/1325c32941139177e6b069e117977dcd0dd30cab).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum closed pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
wangyum closed pull request #29728:
URL: https://github.com/apache/spark/pull/29728
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-692059192
I'm curious why this has to be prohibited - it's not standard SQL? "a = b = c" should be the same as "a = b and b = c"; it's not like it's ambiguous.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691369737
**[Test build #128586 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128586/testReport)** for PR 29728 at commit [`1325c32`](https://github.com/apache/spark/commit/1325c32941139177e6b069e117977dcd0dd30cab).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691417356
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487345912
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691016389
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487862589
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,29 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val isUnsupportedComparison = (left, right) match {
+ case (_: Attribute, _: Predicate) => true
+ case (_: Predicate, _: Attribute) => true
+ case _ => false
+ }
val operator = ctx.comparisonOperator().getChild(0).asInstanceOf[TerminalNode]
operator.getSymbol.getType match {
- case SqlBaseParser.EQ =>
+ case SqlBaseParser.EQ if !isUnsupportedComparison =>
EqualTo(left, right)
- case SqlBaseParser.NSEQ =>
+ case SqlBaseParser.NSEQ if !isUnsupportedComparison =>
EqualNullSafe(left, right)
- case SqlBaseParser.NEQ | SqlBaseParser.NEQJ =>
+ case SqlBaseParser.NEQ | SqlBaseParser.NEQJ if !isUnsupportedComparison =>
Not(EqualTo(left, right))
- case SqlBaseParser.LT =>
+ case SqlBaseParser.LT if !isUnsupportedComparison =>
LessThan(left, right)
- case SqlBaseParser.LTE =>
+ case SqlBaseParser.LTE if !isUnsupportedComparison =>
LessThanOrEqual(left, right)
- case SqlBaseParser.GT =>
+ case SqlBaseParser.GT if !isUnsupportedComparison =>
GreaterThan(left, right)
- case SqlBaseParser.GTE =>
+ case SqlBaseParser.GTE if !isUnsupportedComparison =>
GreaterThanOrEqual(left, right)
+ case _ =>
+ throw new ParseException("Syntax error at or near", ctx)
Review comment:
Could we check this error case at the beginning of this function?
```
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
if (<Error checks>) {
throw new ParseException("Syntax error at or near", ctx)
}
val operator = ctx.comparisonOperator().getChild(0).asInstanceOf[TerminalNode]
operator.getSymbol.getType match {
case SqlBaseParser.EQ =>
EqualTo(left, right)
case SqlBaseParser.NSEQ =>
EqualNullSafe(left, right)
case SqlBaseParser.NEQ | SqlBaseParser.NEQJ =>
Not(EqualTo(left, right))
...
case SqlBaseParser.GTE =>
GreaterThanOrEqual(left, right)
}
```
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,29 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val isUnsupportedComparison = (left, right) match {
Review comment:
How about the case, `select * from t1 where (a > 3) = (b < 5) = (c > 3)` ?
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1327,22 +1327,29 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
override def visitComparison(ctx: ComparisonContext): Expression = withOrigin(ctx) {
val left = expression(ctx.left)
val right = expression(ctx.right)
+ val isUnsupportedComparison = (left, right) match {
Review comment:
btw, have you check if we could update the ANTLR parser rules to prevent this case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a change in pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on a change in pull request #29728:
URL: https://github.com/apache/spark/pull/29728#discussion_r487148070
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlParserSuite.scala
##########
@@ -306,6 +306,15 @@ class SparkSqlParserSuite extends AnalysisTest {
Project(UnresolvedAlias(concat) :: Nil, UnresolvedRelation(TableIdentifier("t"))))
}
+ test("Prohibit binary comparisons chain") {
+ assertEqual("SELECT a = b",
+ Project(UnresolvedAlias(EqualTo(UnresolvedAttribute("a"), UnresolvedAttribute("b"))) :: Nil,
+ OneRowRelation()))
+
+ intercept("SELECT a = b = c", "Syntax error at or near")
Review comment:
Nit: Let's add more cases to cover all the comparisions
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29728: [SPARK-32856][SQL] Prohibit binary comparisons chain
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29728:
URL: https://github.com/apache/spark/pull/29728#issuecomment-691417060
**[Test build #128587 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128587/testReport)** for PR 29728 at commit [`92f1272`](https://github.com/apache/spark/commit/92f1272d16383046fdb260ca4fcafcbda104a6e0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org