Github user cloud-fan closed the pull request at:
https://github.com/apache/spark/pull/4904
---
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 e
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/5189
---
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 enab
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-87479659
LGTM
---
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-87467613
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-87467611
[Test build #29367 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29367/consoleFull)
for PR 5189 at commit
[`b8cae45`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-87454912
[Test build #29367 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29367/consoleFull)
for PR 5189 at commit
[`b8cae45`](https://githu
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86803347
Ah, so `SELECT a.b` is equal to `SELECT a.b AS b`, and the `b` in `ORDER BY
b` should reference to `a.b`, that's how hive does resolution and makes sense.
I have sent y
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86734674
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86734651
[Test build #29255 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29255/consoleFull)
for PR 5189 at commit
[`ca02d85`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86707636
[Test build #29255 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29255/consoleFull)
for PR 5189 at commit
[`ca02d85`](https://githu
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/5189#discussion_r27252191
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala
---
@@ -196,14 +197,19 @@ abstract class LogicalPlan extend
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86685120
@cloud-fan, thanks for the further comments. Regarding not resolving sort
in in `ResolveReferences`, I agree that is a little confusing and I think you
are correct for
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86346279
Another problem, for something like `SELECT DISTINCT a FROM t ORDER BY a`,
we can't resolve this `Sort` because its child is not `Project` or `Aggregate`,
thus can't ha
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/5189#discussion_r27188497
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -340,19 +340,16 @@ class Analyzer(catalog: Catalog,
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86327485
Since we decide to resolve `Sort` in `ResolveSortReferences`, then we need
to stop resolve `Sort` in `ResolveReferences`. For example,
test("quick") {
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86251561
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86251554
[Test build #29187 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29187/consoleFull)
for PR 5189 at commit
[`e35b27c`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5189#issuecomment-86232956
[Test build #29187 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29187/consoleFull)
for PR 5189 at commit
[`e35b27c`](https://githu
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-86232788
Thanks for working on this! This is a pretty tricky case that involves
several rules interacting, and your test cases have been incredibly helpful.
Here are my
GitHub user marmbrus opened a pull request:
https://github.com/apache/spark/pull/5189
[SPARK-6145][SQL] fix ORDER BY on nested fields
This PR is based on work by @cloud-fan in #4904, but with two differences:
- We isolate the logic for Sort's special handling into
`ResolveSortR
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4904#discussion_r27157503
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -257,11 +256,24 @@ class Analyzer(catalog: Catalog,
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4904#discussion_r27157383
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1053,10 +1053,19 @@ class SQLQuerySuite extends QueryTest with
BeforeAndAfte
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4904#discussion_r27148114
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1053,10 +1053,19 @@ class SQLQuerySuite extends QueryTest with
BeforeAndAfte
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4904#discussion_r27148040
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -257,11 +256,24 @@ class Analyzer(catalog: Catalog,
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-85802831
ping @marmbrus
---
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 featu
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-81400936
Hi @marmbrus , any more comments?
---
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 n
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-81383254
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-81383243
[Test build #28634 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28634/consoleFull)
for PR 4904 at commit
[`7b56fb9`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-81351948
[Test build #28634 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28634/consoleFull)
for PR 4904 at commit
[`7b56fb9`](https://githu
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77760988
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77760987
[Test build #28372 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28372/consoleFull)
for PR 4904 at commit
[`eab0b43`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77756133
[Test build #28372 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28372/consoleFull)
for PR 4904 at commit
[`eab0b43`](https://githu
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77756049
Hi @marmbrus , it feels hard for me to resolve the base attribute but not
the GetFields that are on top. When we get into `LogicalPlan#resolve`, the
`Attribute`s are al
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77631396
[Test build #28354 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28354/consoleFull)
for PR 4904 at commit
[`e383154`](https://gith
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77631405
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77625025
[Test build #28354 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28354/consoleFull)
for PR 4904 at commit
[`e383154`](https://githu
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77624293
ok to test
---
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
ena
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/4904#discussion_r25972701
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicOperators.scala
---
@@ -152,6 +153,18 @@ case class Sort(
gl
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77567607
Seems Jenkins doesn't listen to me :(
---
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 d
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/4904#discussion_r25947236
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicOperators.scala
---
@@ -152,6 +153,18 @@ case class Sort(
g
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77566870
retest it please.
---
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 fea
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/4918#issuecomment-77565205
Hi @marmbrus , I studied how we handle ORDER BY and had a more complete fix.
For simple example "SELECT a, b FROM t ORDER BY b", it will be parsed into
S
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4918#discussion_r25921825
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1049,4 +1049,14 @@ class SQLQuerySuite extends QueryTest with
Befor
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/4918#discussion_r25921168
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1049,4 +1049,14 @@ class SQLQuerySuite extends QueryTest with
BeforeAndAf
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4918#discussion_r25920732
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1049,4 +1049,14 @@ class SQLQuerySuite extends QueryTest with
Befor
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/4918#discussion_r25919984
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1049,4 +1049,14 @@ class SQLQuerySuite extends QueryTest with
BeforeAndAf
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4918#discussion_r25918842
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1049,4 +1049,14 @@ class SQLQuerySuite extends QueryTest with
Befor
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4918#discussion_r25918626
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1049,4 +1049,14 @@ class SQLQuerySuite extends QueryTest with
Befor
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4918#discussion_r25918187
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
---
@@ -1049,4 +1049,14 @@ class SQLQuerySuite extends QueryTest with
Befor
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4918
---
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 enab
GitHub user marmbrus opened a pull request:
https://github.com/apache/spark/pull/4918
[SPARK-6145][SQL] fix ORDER BY on nested fields
Based on #4904 with style errors fixed.
`LogicalPlan#resolve` will not only produce `Attribute`, but also
"`GetField` chain".
So in `Res
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4918#issuecomment-77443873
[Test build #28307 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28307/consoleFull)
for PR 4918 at commit
[`997f84e`](https://githu
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77443846
Since I'd really like to include this in the next RC, I've opened #4918
with the style error fixed.
---
If your project is set up for it, you can reply to this email an
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77437670
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77437661
[Test build #28305 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28305/consoleFull)
for PR 4904 at commit
[`3eedbfc`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77437439
[Test build #28305 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28305/consoleFull)
for PR 4904 at commit
[`3eedbfc`](https://githu
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77436434
test this please
---
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 featu
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77347283
Why not review #4892 for me? :)
---
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 doe
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77337425
Hi @chenghao-intel , your test is failed on my code, and I study into it,
here is my thoughts.
First, I think the `LogicalPlan#resolveChildren` logic is not right fo
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77323036
Can you also try the test case:
https://github.com/apache/spark/pull/4892/files#diff-5a2e7f03d14856c8769fd3ddea8742bdR55
The root cause of the failure
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4904#issuecomment-77321983
Can one of the admins verify this patch?
---
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 pro
GitHub user cloud-fan opened a pull request:
https://github.com/apache/spark/pull/4904
[SPARK-6145][SQL] fix ORDER BY on nested fields
`LogicalPlan` will not only produce `Attribute`, but also "GetField chain".
So in `ResolveSortReferences`, after resolve the `ordering` expressio
62 matches
Mail list logo
