[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-12 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-209262219 @cloud-fan The obvious wrong case is Expand's constraints. I've modified the test in `ConstraintPropagationSuite`. Basically the previous usage works except for constrai

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-12 Thread viirya
Github user viirya commented on a diff in the pull request: https://github.com/apache/spark/pull/12138#discussion_r59500928 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicOperators.scala --- @@ -512,24 +512,35 @@ private[sql] object Expand {

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-11 Thread cloud-fan
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208308011 I may misunderstand this problem, could you add a test case in this PR to show what's wrong before? --- If your project is set up for it, you can reply to this email

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-11 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208254696 I think the output attributes of `Expand` operator should based on its projections, not child's output. --- If your project is set up for it, you can reply to this emai

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-11 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208252570 Why? Every time we call `output` method on `Expand`, it still uses child output to construct its output. But child's output doesn't know the values are changed now. Isn'

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-11 Thread cloud-fan
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208247531 > But our output in Expand doesn't reflect this change. It's because `Expand.output` is a `val`, if we make it a `def`, and use `child.output` to construct the out

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-11 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208240385 We may not talk the same thing. Not the change of child output causes problem. We create new attributes in `Expand`. But we use child's output as `Expand`'s output.

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-11 Thread cloud-fan
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208234404 no it's different when we do it in a method. Everytime the child output changes, `Expand.output` will change too. --- If your project is set up for it, you can reply

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-11 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208193457 @cloud-fan As you replace the placeholder with child attributes, does it mean we re-use child's output too? --- If your project is set up for it, you can reply to this

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-10 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208179970 @cloud-fan yea. this solution looks complicated due to that I need to fit it into current usage of `Expand`... I will try your proposal and see if it works. Thanks! --

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-10 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208179409 I just get noticed that this PR can also solve another JIRA ticket: https://issues.apache.org/jira/browse/SPARK-14495. --- If your project is set up for it, you can rep

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-10 Thread cloud-fan
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208168423 Yea, and in this PR we use `NamedExpression` to avoid re-using child's output, which should work. My proposal is that, we can use placeholders(maybe `BoundReference`)

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-10 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-208158392 @cloud-fan Thanks for comment. The problem is we re-use child's output as `Expand`s output. We will create new attributes in `Expand` because `Expand` actually

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-08 Thread cloud-fan
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-207689521 If my understanding is right, the problem is: when child output changes(e.g. making them null when doing a roll up), the output of `Expand` can't reflect it. I have

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-08 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/12138#discussion_r59109073 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicOperators.scala --- @@ -512,24 +512,35 @@ private[sql] object Expan

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-08 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-207662034 ping @marmbrus @yhuai @cloud-fan --- 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 no

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205897396 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 projec

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205897404 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205896549 **[Test build #54993 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54993/consoleFull)** for PR 12138 at commit [`8a18acd`](https://g

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205853906 **[Test build #54993 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54993/consoleFull)** for PR 12138 at commit [`8a18acd`](https://gi

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread viirya
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205851532 retest 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 fe

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205848612 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 projec

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205848622 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205848558 **[Test build #54991 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54991/consoleFull)** for PR 12138 at commit [`8a18acd`](https://g

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread viirya
Github user viirya commented on a diff in the pull request: https://github.com/apache/spark/pull/12138#discussion_r58553204 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala --- @@ -1659,11 +1665,12 @@ object TimeWindowing extends Rule[Log

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-05 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205845316 **[Test build #54991 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54991/consoleFull)** for PR 12138 at commit [`8a18acd`](https://gi

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-04 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205291244 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 projec

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-04 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205291250 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-04 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205290914 **[Test build #54845 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54845/consoleFull)** for PR 12138 at commit [`c9a3887`](https://g

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-04 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-205258643 **[Test build #54845 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54845/consoleFull)** for PR 12138 at commit [`c9a3887`](https://gi

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-03 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-204993643 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 projec

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-03 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-204993644 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-03 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-204993595 **[Test build #54806 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54806/consoleFull)** for PR 12138 at commit [`376d2e1`](https://g

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-03 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12138#issuecomment-204979341 **[Test build #54806 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54806/consoleFull)** for PR 12138 at commit [`376d2e1`](https://gi

[GitHub] spark pull request: [SPARK-14354][SQL] Let Expand take name expres...

2016-04-03 Thread viirya
GitHub user viirya opened a pull request: https://github.com/apache/spark/pull/12138 [SPARK-14354][SQL] Let Expand take name expressions and infer output attributes ## What changes were proposed in this pull request? JIRA: https://issues.apache.org/jira/browse/SPARK-14354