You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by huaxingao <gi...@git.apache.org> on 2018/10/21 16:24:33 UTC
[GitHub] spark pull request #22788: [SPARK-25769][SQL]change nested columns from `a.b...
GitHub user huaxingao opened a pull request:
https://github.com/apache/spark/pull/22788
[SPARK-25769][SQL]change nested columns from `a.b` to `a`.`b`
## What changes were proposed in this pull request?
Currently, ```$"a.b".expr.asInstanceOf[UnresolvedAttribute].sql```
returns
```
res1: String = `a.b`
```
This PR will change the nested column to
```
`a`.`b`
```
## How was this patch tested?
I didn't add any new test. However, I changed several of the existing tests to make them to expect the nested columns to have the format of
```
`a`.`b`
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/huaxingao/spark spark-25769
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22788.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22788
----
commit 2aa4ad97c8b9df5fdd022653549a0252dbb819b6
Author: Huaxin Gao <hu...@...>
Date: 2018-10-21T16:13:49Z
[SPARK-25769][SQL]change nested columns from to .
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22788
This sounds a safe change. cc @liancheng
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98365 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98365/testReport)** for PR 22788 at commit [`693c512`](https://github.com/apache/spark/commit/693c512bdaf66a0c8cfaf513e136d62f36a9b48e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97687 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97687/testReport)** for PR 22788 at commit [`e941acd`](https://github.com/apache/spark/commit/e941acd2b7629ffe0d54b796e1129b02224aef8c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229354247
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
@@ -2856,6 +2856,21 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
checkAnswer(sql("select 26393499451 / (1e6 * 1000)"), Row(BigDecimal("26.3934994510000")))
}
}
+
+ test("SPARK-25769 escape nested columns") {
+ import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute
+ val sql1 = $"a.b".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql1 === "`a`.`b`")
+
+ val sql2 = $"`a.b`".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql2 === "`a.b`")
+
+ val sql3 = $"`a`.b".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql3 === "`a`.`b`")
+
+ val sql4 = $"`a.b`.c".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql4 === "`a.b`.`c`")
+ }
--- End diff --
Merged. Thank you very much! @dongjoon-hyun
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]change nested columns from `a.b...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r226870804
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala ---
@@ -98,8 +98,18 @@ case class UnresolvedTableValuedFunction(
*/
case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute with Unevaluable {
- def name: String =
- nameParts.map(n => if (n.contains(".")) s"`$n`" else n).mkString(".")
+ def name: String = {
+ nameParts.map(n =>
+ if (n.contains(".")) s"`$n`"
+ else {
+ if (nameParts.length > 1) {
+ s"`$n`"
+ } else {
+ n
+ }
+ }
+ ).mkString(".")
+ }
--- End diff --
Hi, @huaxingao . Can we have the following one-line patch instead 12 line changes?
```scala
- nameParts.map(n => if (n.contains(".")) s"`$n`" else n).mkString(".")
+ nameParts.map(n => if (nameParts.length > 1 || n.contains(".")) s"`$n`" else n).mkString(".")
```
cc @cloud-fan and @gatorsmile and @dbtsai .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r227152273
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
@@ -2702,7 +2702,7 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
val e = intercept[AnalysisException](sql("SELECT v.i from (SELECT i FROM v)"))
assert(e.message ==
- "cannot resolve '`v.i`' given input columns: [__auto_generated_subquery_name.i]")
--- End diff --
The out-most backticks is added in
```case _```, in ```case class UnresolvedAttribute(nameParts: Seq[String])```
```
override def sql: String = name match {
case ParserUtils.escapedIdentifier(_) | ParserUtils.qualifiedEscapedIdentifier(_, _) => name
case _ => quoteIdentifier(name)
}
```
the ```nameParts``` is a Seq of ```"v" "i"```, name is ```v.i```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98366/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97687/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4372/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98366 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98366/testReport)** for PR 22788 at commit [`05688f5`](https://github.com/apache/spark/commit/05688f5c2e4bea28e3759096755bc979d470d004).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98003 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98003/testReport)** for PR 22788 at commit [`458f77a`](https://github.com/apache/spark/commit/458f77a6b07d001bd06e37ed2f1fbc0c8beecf0a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/22788
@huaxingao Can you update the PR description based on @dongjoon-hyun's comments https://github.com/apache/spark/pull/22788#pullrequestreview-166797320. It is more clear on the improvement of this change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229578738
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
@cloud-fan I had a question, the 3rd example from huaxin, wanted to confirm again if the output looks okay to you.
**from huaxin**
```
$"`a`.b".expr.asInstanceOf[org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute].sql
```
with my previous fix:
```
`a`.`b`
```
make sql same as name:
```
a.b
```
Is it okay to drop the backtick from the first identifier ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98376 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98376/testReport)** for PR 22788 at commit [`3c81840`](https://github.com/apache/spark/commit/3c81840f80432c4d341bf94ce80a399c43a0ef4e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98270 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98270/testReport)** for PR 22788 at commit [`7ff2696`](https://github.com/apache/spark/commit/7ff26964ab9e47dc7598feb9aae0896126ab871b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4703/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22788
@viirya . Please see the all four examples. I guess you missed the context.
- https://github.com/apache/spark/pull/22788#pullrequestreview-166797320
BTW, I'm good for any methods if we can proceed further, @cloud-fan and @dbtsai .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]change nested columns from `a.b...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r226987977
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -81,7 +81,7 @@ SELECT t1.i1 FROM t1, mydb1.t1
struct<>
-- !query 9 output
org.apache.spark.sql.AnalysisException
-Reference 't1.i1' is ambiguous, could be: mydb1.t1.i1, mydb1.t1.i1.; line 1 pos 7
+Reference '`t1`.`i1`' is ambiguous, could be: mydb1.t1.i1, mydb1.t1.i1.; line 1 pos 7
--- End diff --
why is the new format better? it's more verbose, isn't it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r227202767
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala ---
@@ -99,7 +99,7 @@ case class UnresolvedTableValuedFunction(
case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute with Unevaluable {
def name: String =
- nameParts.map(n => if (n.contains(".")) s"`$n`" else n).mkString(".")
+ nameParts.map(n => if (nameParts.length > 1 || n.contains(".")) s"`$n`" else n).mkString(".")
--- End diff --
I don't think this is better for `name`, we should update `sql` though.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97881 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97881/testReport)** for PR 22788 at commit [`99bfd00`](https://github.com/apache/spark/commit/99bfd0099378200e606293368c2914d5628adc44).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/22788
@huaxingao Can you update the PR description based on @dongjoon-hyun's comments https://github.com/apache/spark/pull/22788#pullrequestreview-166797320. It is more clear on the improvement of this change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229538040
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229585323
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
> Is it okay to drop the backtick from the first identifier
AFAIK both `name` and `sql` are for display/message. I think dropping backtick is fine if no ambiguous
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4704/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97881 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97881/testReport)** for PR 22788 at commit [`99bfd00`](https://github.com/apache/spark/commit/99bfd0099378200e606293368c2914d5628adc44).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r228781145
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
do you think we should just make `sql` same as `name`? It looks to me that `'db1.t1.i1'` is better than `` '`db1`.`t1`.`i1`' ``, as it's more compact and is not ambiguous.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4711/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4182/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97881/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4377/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229577806
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
@huaxingao Were you planning to make a change to make `name` same as `sql` ? Currently they are different ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/22788
@dongjoon-hyun Oh I see. The ambiguousness is in the results of `sql` for several inputs.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22788
yea, only use json if it's a nested column.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4465/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]change nested columns from `a.b...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r226988117
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
@@ -2702,7 +2702,7 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
val e = intercept[AnalysisException](sql("SELECT v.i from (SELECT i FROM v)"))
assert(e.message ==
- "cannot resolve '`v.i`' given input columns: [__auto_generated_subquery_name.i]")
--- End diff --
I think the problem here is the out-most backticks, do you know where we add it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98003 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98003/testReport)** for PR 22788 at commit [`458f77a`](https://github.com/apache/spark/commit/458f77a6b07d001bd06e37ed2f1fbc0c8beecf0a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/22788
@huaxingao Can you update the PR description based on @dongjoon-hyun's comments https://github.com/apache/spark/pull/22788#pullrequestreview-166797320. It is more clear on the improvement of this change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22788
I agree with the problem described in the PR description that `UnresolvedAttribute.sql` is not ideal. But we should just update `UnresolvedAttribute.sql`, not the `name` method. `name` is used in other places and I think it has no problem.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97686 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97686/testReport)** for PR 22788 at commit [`2aa4ad9`](https://github.com/apache/spark/commit/2aa4ad97c8b9df5fdd022653549a0252dbb819b6).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97686/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98270 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98270/testReport)** for PR 22788 at commit [`7ff2696`](https://github.com/apache/spark/commit/7ff26964ab9e47dc7598feb9aae0896126ab871b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97877 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97877/testReport)** for PR 22788 at commit [`99bfd00`](https://github.com/apache/spark/commit/99bfd0099378200e606293368c2914d5628adc44).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4629/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229583440
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
@dilipbiswal
Currently ```sql``` is not the same as ```name```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98376/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97877 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97877/testReport)** for PR 22788 at commit [`99bfd00`](https://github.com/apache/spark/commit/99bfd0099378200e606293368c2914d5628adc44).
* This patch **fails MiMa tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22788
Retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98365 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98365/testReport)** for PR 22788 at commit [`693c512`](https://github.com/apache/spark/commit/693c512bdaf66a0c8cfaf513e136d62f36a9b48e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22788
@cloud-fan I like the idea of using JSON, but that will also change the definition of string format. Do we just use JSON for nested case so the existing data source doesn't have to be changed?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/22788
From above examples,
```scala
scala> $"`a.b`".expr.asInstanceOf[org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute].sql
res1: String = `a.b` // ambiguous
```
Is this ambiguous? Doesn't it mean a column simply named `a.b`? Btw I don't see the difference between before and after this change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]change nested columns from `a.b...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r227093538
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -81,7 +81,7 @@ SELECT t1.i1 FROM t1, mydb1.t1
struct<>
-- !query 9 output
org.apache.spark.sql.AnalysisException
-Reference 't1.i1' is ambiguous, could be: mydb1.t1.i1, mydb1.t1.i1.; line 1 pos 7
+Reference '`t1`.`i1`' is ambiguous, could be: mydb1.t1.i1, mydb1.t1.i1.; line 1 pos 7
--- End diff --
I thought some examples in [the above](https://github.com/apache/spark/pull/22788#pullrequestreview-166797320).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98270/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229797016
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
@cloud-fan Makes sense. Thanks for clarification...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22788
Yea I think so, we can even use JSON to be safer. e.g. for `a.b.c.d`, we can encode it as a json array [a,b,c,d]. At data source side, use a json parser to read it back.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98365/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98366 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98366/testReport)** for PR 22788 at commit [`05688f5`](https://github.com/apache/spark/commit/05688f5c2e4bea28e3759096755bc979d470d004).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on the issue:
https://github.com/apache/spark/pull/22788
I have a question regarding the test failure in ```ExpressionTypeCheckingSuite```. Most of the tests in this suite failed after I change ```UnresolvedAttribute.sql = UnresolvedAttribute.name```. For example,
```
test("check types for unary arithmetic") {
assertError(BitwiseNot('stringField), "requires integral type")
}
```
The test failed with
```
"cannot resolve '~`stringField`' due to data type mismatch: argument 1 requires integral
type, however, '`stringField`' is of string type.;" did not contain "cannot resolve
'~stringField' due to data type mismatch:"
```
It seems that the root cause of the failure is that ```UnresolveAttribute.sql``` doesn't match with ```AttributeReference.sql``` any more after my fix. I am in doubt if it is correct to make ```UnresolvedAttribute.sql ``` the same as ``` UnresolvedAttribute.name```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97686 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97686/testReport)** for PR 22788 at commit [`2aa4ad9`](https://github.com/apache/spark/commit/2aa4ad97c8b9df5fdd022653549a0252dbb819b6).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #98376 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98376/testReport)** for PR 22788 at commit [`3c81840`](https://github.com/apache/spark/commit/3c81840f80432c4d341bf94ce80a399c43a0ef4e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4183/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]make UnresolvedAttribute.sql escape ne...
Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on the issue:
https://github.com/apache/spark/pull/22788
@cloud-fan @dongjoon-hyun
Because of the above test failures in ```ExpressionTypeCheckingSuite```, shall I revert to the previous change ?
```
override def sql: String = nameParts.map { part =>
part match {
case ParserUtils.escapedIdentifier(_) | ParserUtils.qualifiedEscapedIdentifier(_, _) =>
part
case _ =>
quoteIdentifier(part)
}
}.mkString(".")
```
Or change ```ExpressionTypeCheckingSuite```?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98003/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97877/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r229479893
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -161,7 +161,7 @@ SELECT db1.t1.i1 FROM t1, mydb2.t1
struct<>
-- !query 18 output
org.apache.spark.sql.AnalysisException
-cannot resolve '`db1.t1.i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
+cannot resolve '`db1`.`t1`.`i1`' given input columns: [mydb2.t1.i1, mydb2.t1.i1]; line 1 pos 7
--- End diff --
Yes. I agree. For the four examples, we will have the following results:
```
$"a.b".expr.asInstanceOf[org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute].sql
```
with my previous fix:
```
`a`.`b`
```
make sql same as name:
```
a.b
```
```
$"`a.b`".expr.asInstanceOf[org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute].sql
```
with my previous fix:
```
`a`.`b`
```
make sql same as name:
```
`a.b`
```
```
$"`a`.b".expr.asInstanceOf[org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute].sql
```
with my previous fix:
```
`a`.`b`
```
make sql same as name:
```
a.b
```
```
$"`a.b`.c".expr.asInstanceOf[org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute].sql
```
with my previous fix:
```
`a.b`.`c`
```
make sql same as name:
```
`a.b`.c
```
Does this look good to everybody?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r228770171
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
@@ -2856,6 +2856,21 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
checkAnswer(sql("select 26393499451 / (1e6 * 1000)"), Row(BigDecimal("26.3934994510000")))
}
}
+
+ test("SPARK-25769 escape nested columns") {
+ import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute
+ val sql1 = $"a.b".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql1 === "`a`.`b`")
+
+ val sql2 = $"`a.b`".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql2 === "`a.b`")
+
+ val sql3 = $"`a`.b".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql3 === "`a`.`b`")
+
+ val sql4 = $"`a.b`.c".expr.asInstanceOf[UnresolvedAttribute].sql
+ assert(sql4 === "`a.b`.`c`")
+ }
--- End diff --
Hi, @huaxingao . Can we move this test to `ColumnExpressionSuite`? I made a [PR](https://github.com/huaxingao/spark/pull/1) to you. Please review and merge.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]escape nested columns by backti...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r227203199
--- Diff: sql/core/src/test/resources/sql-tests/results/columnresolution-negative.sql.out ---
@@ -81,7 +81,7 @@ SELECT t1.i1 FROM t1, mydb1.t1
struct<>
-- !query 9 output
org.apache.spark.sql.AnalysisException
-Reference 't1.i1' is ambiguous, could be: mydb1.t1.i1, mydb1.t1.i1.; line 1 pos 7
+Reference '`t1`.`i1`' is ambiguous, could be: mydb1.t1.i1, mydb1.t1.i1.; line 1 pos 7
--- End diff --
These examples only make sense when we have the outer backticks. e.g. `'t1.i1'` is good.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]change nested columns from `a.b` to `a...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22788
**[Test build #97687 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97687/testReport)** for PR 22788 at commit [`e941acd`](https://github.com/apache/spark/commit/e941acd2b7629ffe0d54b796e1129b02224aef8c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:
https://github.com/apache/spark/pull/22788
@cloud-fan @dongjoon-hyun instead of changing `Filter` API, do you think using proper escaped char like this PR in https://github.com/apache/spark/pull/22573 is a good approach?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22788: [SPARK-25769][SQL]change nested columns from `a.b...
Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:
https://github.com/apache/spark/pull/22788#discussion_r226872842
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala ---
@@ -98,8 +98,18 @@ case class UnresolvedTableValuedFunction(
*/
case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute with Unevaluable {
- def name: String =
- nameParts.map(n => if (n.contains(".")) s"`$n`" else n).mkString(".")
+ def name: String = {
+ nameParts.map(n =>
+ if (n.contains(".")) s"`$n`"
+ else {
+ if (nameParts.length > 1) {
+ s"`$n`"
+ } else {
+ n
+ }
+ }
+ ).mkString(".")
+ }
--- End diff --
@dongjoon-hyun Thanks! Fixed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22788: [SPARK-25769][SQL]escape nested columns by backtick each...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22788
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org