You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kevinyu98 <gi...@git.apache.org> on 2016/02/02 01:46:01 UTC
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
GitHub user kevinyu98 opened a pull request:
https://github.com/apache/spark/pull/11009
[SPARK-12987][SQL]Fixing the name resolution in drop column
@marmbrus @cloud-fan @thomas @jayadevan : Hello All: Can you help review this code fix?
This problem is coming from drop column, after we drop off the old column, construct the new dataframe from the remaining columns.
the new dataframe is using the Schema information to construct the column name, in this case, the name is string ‘a.c’. Since it is from Schema, we should take the name as it is, we should not do any parsing on the name.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kevinyu98/spark work_on_spark-12987
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11009.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 #11009
----
commit ff187c38234686ede0f859f4fe00a8013d8dc86f
Author: Kevin Yu <qy...@us.ibm.com>
Date: 2016-02-02T00:08:01Z
Fixing the name resolution in drop column
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by kevinyu98 <gi...@git.apache.org>.
GitHub user kevinyu98 reopened a pull request:
https://github.com/apache/spark/pull/11009
[SPARK-12987][SQL]Fixing the name resolution in drop column
@marmbrus @cloud-fan @thomas @jayadevan : Hello All: Can you help review this code fix?
This problem is coming from drop column, after we drop off the old column, construct the new dataframe from the remaining columns.
the new dataframe is using the Schema information to construct the column name, in this case, the name is string ‘a.c’. Since it is from Schema, we should take the name as it is, we should not do any parsing on the name.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kevinyu98/spark work_on_spark-12987
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11009.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 #11009
----
commit ff187c38234686ede0f859f4fe00a8013d8dc86f
Author: Kevin Yu <qy...@us.ibm.com>
Date: 2016-02-02T00:08:01Z
Fixing the name resolution in drop column
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-179513155
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50686/
Test PASSed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-179513148
Merged build finished. Test PASSed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178279335
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 project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-179478292
@marmbrus @cloud-fan @dilipbiswal @yzhou2001 : I have change the code based on Michael's comments, can you help review it again?
Not sure why the first test failed, I run the sql test locally, it passed.
[info] Run completed in 3 minutes, 23 seconds.
[info] Total number of tests run: 1553
[info] Suites: completed 110, aborted 0
[info] Tests: succeeded 1553, failed 0, canceled 0, ignored 10, pending 0
[info] All tests passed.
[info] Passed: Total 1553, Failed 0, Errors 0, Passed 1553, Ignored 10
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-179477437
**[Test build #50686 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50686/consoleFull)** for PR 11009 at commit [`fb59a8a`](https://github.com/apache/spark/commit/fb59a8a43ce124eb494f840d967f89810fed8f7f).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178712237
@marmbrus @cloud-fan @dilipbiswal @yzhou2001 : it seems this is the duplicate of [SPARK-12988][SQL] Can't drop columns that contain dots #10943, I will close this PR. thanks.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/11009#discussion_r51509060
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrame.scala ---
@@ -1257,7 +1257,8 @@ class DataFrame private[sql](
def drop(colNames: String*): DataFrame = {
val resolver = sqlContext.analyzer.resolver
val remainingCols =
- schema.filter(f => colNames.forall(n => !resolver(f.name, n))).map(f => Column(f.name))
+ schema.filter(f => colNames.forall(n => !resolver(f.name, n)))
+ .map(f => Column(UnresolvedAttribute(Seq(f.name))))
--- End diff --
I would do `UnresolvedAttribute.quoted(f.name)`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178319870
**[Test build #50515 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50515/consoleFull)** for PR 11009 at commit [`ff187c3`](https://github.com/apache/spark/commit/ff187c38234686ede0f859f4fe00a8013d8dc86f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178281031
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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178357731
is it duplicated with https://github.com/apache/spark/pull/10943?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #11009: [SPARK-12987][SQL]Fixing the name resolution in drop col...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/11009
Thanks for the pull request. I'm going through a list of pull requests to cut them down since the sheer number is breaking some of the tooling we have. Due to lack of activity on this pull request, I'm going to push a commit to close it. Feel free to reopen it or create a new one. We can also continue the discussion on the JIRA ticket.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/11009#discussion_r51509057
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrame.scala ---
@@ -1257,7 +1257,8 @@ class DataFrame private[sql](
def drop(colNames: String*): DataFrame = {
val resolver = sqlContext.analyzer.resolver
val remainingCols =
- schema.filter(f => colNames.forall(n => !resolver(f.name, n))).map(f => Column(f.name))
+ schema.filter(f => colNames.forall(n => !resolver(f.name, n)))
--- End diff --
super nit: i would consider breaking before the filter also if you have to break for the map. otherwise the filter looks special when really you are just chaining a bunch of operations.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178320218
Merged build finished. Test FAILed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 closed the pull request at:
https://github.com/apache/spark/pull/11009
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by yzhou2001 <gi...@git.apache.org>.
Github user yzhou2001 commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178359731
The two jiras seem to be identical; but the fixes are different.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178362481
@cloud-fan Yeah.. 10943 also fixes the same issue.
@yzhou2001 Actually the original fix was sort of similar to fix in this PR except that it also considered the drop(Column) interface. I have been trying to improve the fix based on suggestion from Wenchen :-)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178286597
**[Test build #50515 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50515/consoleFull)** for PR 11009 at commit [`ff187c3`](https://github.com/apache/spark/commit/ff187c38234686ede0f859f4fe00a8013d8dc86f).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #11009: [SPARK-12987][SQL]Fixing the name resolution in d...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/11009
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178320220
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50515/
Test FAILed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-179512208
**[Test build #50686 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50686/consoleFull)** for PR 11009 at commit [`fb59a8a`](https://github.com/apache/spark/commit/fb59a8a43ce124eb494f840d967f89810fed8f7f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12987][SQL]Fixing the name resolution i...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/11009#issuecomment-178738113
Actually, I'd really like to include this in 1.6.1 and this is a nice small fix. Could you reopen?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org