[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/16564 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/16564#discussion_r9636
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
---
@@ -898,11 +899,15 @@ class DatasetSuite extends QueryTest with
SharedSQLContext {
(1, 2), (1, 1), (2, 1), (2, 2))
}
- test("dropDuplicates should not change child plan output") {
-val ds = Seq(("a", 1), ("a", 2), ("b", 1), ("a", 1)).toDS()
-checkDataset(
- ds.dropDuplicates("_1").select(ds("_1").as[String],
ds("_2").as[Int]),
- ("a", 1), ("b", 1))
+ test("SPARK-19065 dropDuplicates should not create expressions using the
same id") {
--- End diff --
It seems weird to me that adding a test to verify that we don't support
some feature, so I just added my previous regression test back in order to have
a test to catch this issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/16564#discussion_r96108323
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
---
@@ -898,11 +899,15 @@ class DatasetSuite extends QueryTest with
SharedSQLContext {
(1, 2), (1, 1), (2, 1), (2, 2))
}
- test("dropDuplicates should not change child plan output") {
-val ds = Seq(("a", 1), ("a", 2), ("b", 1), ("a", 1)).toDS()
-checkDataset(
- ds.dropDuplicates("_1").select(ds("_1").as[String],
ds("_2").as[Int]),
- ("a", 1), ("b", 1))
+ test("SPARK-19065 dropDuplicates should not create expressions using the
same id") {
--- End diff --
oh I didn't mean to test this bug, but the behavior change, i.e. what you
discussed with Michael:
https://github.com/apache/spark/pull/16564#issuecomment-272298726
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/16564#discussion_r96053927
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
---
@@ -898,11 +899,15 @@ class DatasetSuite extends QueryTest with
SharedSQLContext {
(1, 2), (1, 1), (2, 1), (2, 2))
}
- test("dropDuplicates should not change child plan output") {
-val ds = Seq(("a", 1), ("a", 2), ("b", 1), ("a", 1)).toDS()
-checkDataset(
- ds.dropDuplicates("_1").select(ds("_1").as[String],
ds("_2").as[Int]),
- ("a", 1), ("b", 1))
+ test("SPARK-19065 dropDuplicates should not create expressions using the
same id") {
--- End diff --
This test doesn't work. The test needs to trigger an attribute replacement
to expose this bug.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/16564#discussion_r95979368
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
---
@@ -898,11 +899,15 @@ class DatasetSuite extends QueryTest with
SharedSQLContext {
(1, 2), (1, 1), (2, 1), (2, 2))
}
- test("dropDuplicates should not change child plan output") {
-val ds = Seq(("a", 1), ("a", 2), ("b", 1), ("a", 1)).toDS()
-checkDataset(
- ds.dropDuplicates("_1").select(ds("_1").as[String],
ds("_2").as[Int]),
- ("a", 1), ("b", 1))
+ test("SPARK-19065 dropDuplicates should not create expressions using the
same id") {
--- End diff --
how about we remove this test and add a new test to show the behavior
change more obvious?
```
val df = ...
val df2 = df.dropDuplicates("i")
intercept[AnalysisException] { df2.select(df("i")) }
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/16564#discussion_r95942753
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
---
@@ -898,11 +899,15 @@ class DatasetSuite extends QueryTest with
SharedSQLContext {
(1, 2), (1, 1), (2, 1), (2, 2))
}
- test("dropDuplicates should not change child plan output") {
-val ds = Seq(("a", 1), ("a", 2), ("b", 1), ("a", 1)).toDS()
-checkDataset(
- ds.dropDuplicates("_1").select(ds("_1").as[String],
ds("_2").as[Int]),
- ("a", 1), ("b", 1))
+ test("SPARK-19065 dropDuplicates should not create expressions using the
same id") {
--- End diff --
This may introduce other unknown issues because I saw right now SQL rules
that replace attributes don't deal with `Alias`s.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/16564#discussion_r95942576
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
---
@@ -898,11 +899,15 @@ class DatasetSuite extends QueryTest with
SharedSQLContext {
(1, 2), (1, 1), (2, 1), (2, 2))
}
- test("dropDuplicates should not change child plan output") {
-val ds = Seq(("a", 1), ("a", 2), ("b", 1), ("a", 1)).toDS()
-checkDataset(
- ds.dropDuplicates("_1").select(ds("_1").as[String],
ds("_2").as[Int]),
- ("a", 1), ("b", 1))
+ test("SPARK-19065 dropDuplicates should not create expressions using the
same id") {
--- End diff --
It's in my fist commit:
https://github.com/apache/spark/pull/16564/commits/13f54a93c0cf31a38455e90aec722e890af980c6
I removed it because it's not a Structured Streaming issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #16564: [SPARK-19065][SQL]Don't inherit expression id in ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/16564#discussion_r95938778
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
---
@@ -898,11 +899,15 @@ class DatasetSuite extends QueryTest with
SharedSQLContext {
(1, 2), (1, 1), (2, 1), (2, 2))
}
- test("dropDuplicates should not change child plan output") {
-val ds = Seq(("a", 1), ("a", 2), ("b", 1), ("a", 1)).toDS()
-checkDataset(
- ds.dropDuplicates("_1").select(ds("_1").as[String],
ds("_2").as[Int]),
- ("a", 1), ("b", 1))
+ test("SPARK-19065 dropDuplicates should not create expressions using the
same id") {
--- End diff --
do you have an end-to-end test to show that using same id when alias will
cause troubles?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
