[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @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 not have this feature enabled and wishes so, or if the

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 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 feat

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 I've actually started digging in this direction because I've encountered the opposite case with an older spark version, CreateStruct lost attributes names after CleanupAliases operated on it, unfortu

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan,how would you like to proceed? --- 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 en

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 I'd like to make `CreateStruct` just a place holder, and create a new rule in `Analyzer` to always convert `CreateStruct` to `CreateNamedStruct` --- If your project is set up for it, you can repl

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 As a separate till running before CleanupAliases? I'll sketch something later today. On Aug 3, 2016 04:07, "Wenchen Fan" wrote: > I'd like to make CreateStruct just a place holde

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @jaceklaskowski this commit should address most of your comments, I've answered to the rest of them. --- If your project is set up for it, you can reply to this email and have your reply appear on G

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan , is this what you meant by creating a new rule in Analyzer? I've also introduced a new batch for this rule, executed only once as a single pass over the tree should be sufficient - a s

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 I'd like to make `CreateStruct` extends `Unevaluable` and remove the evaluation logic. Then we can have a rule `ResolveCreateStruct` and put it in batch `Resolution`. --- If your project is set

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 So basically two action items: 1. Modify createStruct and createStructUnsafe to 'loose' their runtime and code generation capabilities. 2. Rename the introduced rule. Btw is it OK to execut

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 `execute it once` is not that important for this simple rule, I think it's ok to put it in Resolution batch --- If your project is set up for it, you can reply to this email and have your reply a

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan this turned out to be a bit wider than I thought it is, I had to touch few areas: 1. ExpressionEncoder, (seems to be your craft) 2. AstBuilder, (@hvanhovell) 3. CreateStruct, Cr

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 Why? I think we only need to touch `Analyzer` and remove the execution logic from `CreateStruct`. Wherever we create `CreateStruct`, we just keep it, but replace(or resolve) it in Analyzer. ---

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 I've executed the entire test suits tree of the catalyst projects and seen failures. Some of these fail, the most notables are the ast-builder and expression-encoder that failed on attempts

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 First (hopefully dumb) question: why is this needed? Maybe I am missing something: Shouldn't we just ban CreateStruct (and its unsafe brother)? Why keep these around if we are going to re

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 I think we need createStruct first of all for backward compatibility and second because some constructs (such as row creator) maps directly into its structure. I think that the way ast-buil

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 @eyalfa we don't need to maintain backwards compatibility for `CreateStruct`, and we can create a factory function for row creator. So I would just remove both `Create*Struct` classes. T

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan , I know this has been idle for few weeks now but I'd like to push it forward. I've merged master into it, which eliminated the discussion about ast-builder and inline-tables (thanks t

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell, are you suggesting a complete removal of CreateStruct? this can definitely simplify this PR, but it'd still leave the issue I've described at the end of my previous comment, one can

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell , I'm currently trying your approach of testing `ne.resolved` prior to accessing `ne.name`. tests are running as I write here, but a quick dive into the `NamedExpression` hierarchy re

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan, please see this [https://github.com/apache/spark/blob/1dbb725dbef30bf7633584ce8efdb573f2d92bca/sql/core/src/main/scala/org/apache/spark/sql/functions.scala#L1104-L1115](url), it seems tha

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 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 feature enabled and wishes so,

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65600 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65600/consoleFull)** for PR 1 at commit [`3360e8c`](https://github.com/apache/spark/commit/3

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65600 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65600/consoleFull)** for PR 1 at commit [`3360e8c`](https://github.com/apache/spark/commit/

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 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 e

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65600/ Test FAILed. ---

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan can you please review the fix and retest? --- 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

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 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 feature enabled and wishes so

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65613 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65613/consoleFull)** for PR 1 at commit [`abf7712`](https://github.com/apache/spark/commit/a

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65613 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65613/consoleFull)** for PR 1 at commit [`abf7712`](https://github.com/apache/spark/commit/

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 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 e

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65613/ Test FAILed. ---

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan, @hvanhovell, test failures assured my concerns about not using the names of unresolved named expressions, it turns out there are cases where the name is available for unresolved expre

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan, @hvanhovell, any update on tis? did you get to review this? --- 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 pr

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan, @hvanhovell , it's waiting for your review over a week now. please comment --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as we

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan, @hvanhovell , I'd really appreciate your comments on this as it really is close to completion. please let me know if you prefer a new pull request (easier to review without the merge

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 @eyalfa I would favor removing the `Create*Struct` classes altogether. Could you elaborate more on how this connects to `Encoders`? @cloud-fan what is your take on this? --- If your pro

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell , if you examine the diffs in this pr, you'll see that ExpressionEncoder [uses](https://github.com/apache/spark/pull/1/files#diff-91c617f2464cea010922328f4cdbbda9R136) CreateStruct t

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell, after a quick 'find references' on CreateStruct it seems that there are many places whre its constructor being used, but only numerous places where it's being pattern-matched. I'm th

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell,please have a look --- 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 w

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell,can you please have a look at this? --- 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 feat

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan , @hvanhovell , can you please review this? it's been waiting for some time now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 @eyalfa there are IMO still two more things to do: 1. Get rid of the `CreateStructLikeFactory`; that is way to complicated for what you are trying to do. 2. Touch up the tests in orde

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 Can you suggest an other approach to prevent duplication of the code that constructs the 'named' arguments? On Sep 3, 2016 19:16, "Herman van Hovell" wrote: > @eyalfa

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 Something like this: ```scala object CreateNamedStruct { def nameColumns(expressions: Seq[Expression]): Seq[Expression] = { expressions.zipWithIndex.flatMap { case (

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 thanks, I think I came up with something simpler, I've implemented the logic in CreateStruct and reused it for CreateStructUnsafe by calling CreateStruct(children), then extracting the new children a

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 Why not share the method for creating the named children, and use that in CreateUnsafeStruct? --- If your project is set up for it, you can reply to this email and have your reply appear on GitH

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 that was basically the idea behind the trait you requested me to remove... the thing is I don't feel it belongs to CreateNamedStruct, so I settled for putting it in CreateStruct and reusing it in

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 I didn't like the trait because it added another layer of complexity (inheritance) to a relatively simple problem. The only thing you need is a shared method. That can be located anywhere.

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 after having a second look at it, it seems CreateStructUnsafe is only used in test code. @hvanhovell do you want to eliminate it all-together and dismiss the entire discussion about code reuse?

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 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 feature enabled and wishes so,

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 mostly LGTM, can you also address this? https://github.com/apache/spark/pull/1/files#r77588295 --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65068 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65068/consoleFull)** for PR 1 at commit [`d6fbe2a`](https://github.com/apache/spark/commit/d

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65068 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65068/consoleFull)** for PR 1 at commit [`d6fbe2a`](https://github.com/apache/spark/commit/

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 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 e

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65068/ Test FAILed. ---

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan what exactly did you want me to address? asides I've removed the redundant annotation from CreateStruct.appl. ...looking into test failures --- If your project is set up for i

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloudfan, seems that the answer file "subquery_in_having_2" has to be generated again. can you assist with that as the hive tests are virtually un-runnable on a windows env. * I had to

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 we can update the `spark/sql/hive/src/test/resources/sqlgen/subquery_in_having_2.sql` file directly --- If your project is set up for it, you can reply to this email and have your reply appear o

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 yes, I thought about it, this is a one-liner modification that canbe copy-pasted from the failure, I'll do it tonight. --- If your project is set up for it, you can reply to this email and have your

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan, @hvanhovell , can someone help me with this failure? I'm not acquainted to R so I don't even know where to start looking for this error. --- If your project is set up for it, you can rep

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 @HyukjinKwon any help? --- 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 wishe

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell Thanks for cc'ing me. I will try to look closely within coming few days. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as we

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user shivaram commented on the issue: https://github.com/apache/spark/pull/1 The easiest way to debug the R test case is to see what has changed for this particular `select` query that is failing and then update the test case. If you have compiled with `-Psparkr` th

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 I see. It seems using `struct(...)` does not print `struct(...)` but `named_struct(...)` as specified in `CreateNamedStruct`. So, the code below: ```scala scala> spark.range

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 cc @shivaram Would this be sensible if we print the results if R tests 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

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @HyukjinKwon, thank you very much for your analysis. if you read the history of this PR you'd see that at some point @hvanhovell suggested that we completely remove CreateStruct and CreateStructUn

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user shivaram commented on the issue: https://github.com/apache/spark/pull/1 @eyalfa Yes - we get the schema from Scala and assign the column names from that [1]. I think @hvanhovell knows more about the compatibility requirements Also @HyukjinKwon if there is a wa

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 I took a look but it seems there is no way without changing it. There is an option `info` to prints but it seems it prints everytime whether it fails the tests or not. Maybe we could try latter.

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @shivaram, thanks for your quick answer. in this case, this particular failure can be remedied by aliasing the columns, right? --- If your project is set up for it, you can reply to this email a

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 @eyalfa Do you mind if I ask whether this is intended to print `named_struct(..)` from calling `struct(..)`? --- If your project is set up for it, you can reply to this email and have your rep

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @HyukjinKwon ,this pr started with a 'rewrite' of struct into named_struct (in order to fix an analysis bug), it later evolved into a complete removal of CreateStruct. I guess we did think about the

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 I think they missed the print representation. Changing internal logics is fine but I don't think internal optimization should affect what users face. Do you mind if I ask clarify here please - @

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/1 This happens if you don't provide an alias for the expression. To me this is not really a backwards compatibility issue. Yes, the default alias is changed, but think of the use case for t

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 @hvanhovell I was just wondering if it is intended or not and just wanted to avoid a mistake for my future PRs. I am fine with all and would like to stay neutral. Thank you for your kind explana

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @HyukjinKwon, I'd appreciate if you could upload a patch with the fix, will save me from pondering in unfamiliar landscape :-) --- If your project is set up for it, you can reply to this email and h

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 Just locally tested. Fixing [here](https://github.com/eyalfa/spark/blob/4cbb9a52e532a40449bc3038cd6bf4c4c3bad82d/R/pkg/inst/tests/testthat/test_sparkSQL.R#L1168-L1177) as below will be fine:

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @HyukjinKwon, once signed off, can you either post a patch file or a url I can merge from? thanks, Eyal. --- If your project is set up for it, you can reply to this email and have your

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 I just opened a pull request against your forked repo - https://github.com/eyalfa/spark/pull/1 :) --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @HyukjinKwon thanks, looking at it as we speak :-) can I merge it, or is it still pending for sign-off by @shivaram? --- If your project is set up for it, you can reply to this email and have

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/1 I think it'd be okay to merge it? If there are some more fixes to be done, I will post another pull request :). I am pretty much always online. --- If your project is set up for it, you can rep

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user shivaram commented on the issue: https://github.com/apache/spark/pull/1 Yeah the R change looks fine. --- 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] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user shivaram commented on the issue: https://github.com/apache/spark/pull/1 Jenkins, 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 feature enabled and wi

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65377 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65377/consoleFull)** for PR 1 at commit [`c1d5b20`](https://github.com/apache/spark/commit/c

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 thanks, @shivaram :-) --- 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

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 Sorry I missed this external change. I agree with @hvanhovell that the default column name is not really a backwards compatibility issue, but I'm also happy if we can fix it without a lot effort.

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloudfan, given that CreateNamedStruct is used in pattern matching quite much, I think adding such a flag might introduce too much noise. I think we can explore the possibility of adding a second ar

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65377 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65377/consoleFull)** for PR 1 at commit [`c1d5b20`](https://github.com/apache/spark/commit/

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 ok that's fine, LGTM then --- 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 wis

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 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 e

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65377/ Test FAILed. ---

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan can you restart the build? pushed a fix --- 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 f

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 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 feature enabled and wishes so,

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65388 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65388/consoleFull)** for PR 1 at commit [`f4f3de4`](https://github.com/apache/spark/commit/f

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/1 **[Test build #65388 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65388/consoleFull)** for PR 1 at commit [`f4f3de4`](https://github.com/apache/spark/commit/

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65388/ Test FAILed. ---

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/1 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 e

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 @cloud-fan, @hvanhovell , do you have an idea for solving the failing test? two possibilities I can think about: 1. CreateStruct.apply should have a special case for Star expression, basic

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user eyalfa commented on the issue: https://github.com/apache/spark/pull/1 I'm not talking about the default column name,but rather on explicit column names. When users alias the expression or refers to already named columns (I.e. table columns) they expect the strut'

[GitHub] spark issue #14444: [SPARK-16839] [SQL] redundant aliases after cleanupAlias...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/1 As we have `struct` and `name_struct`, I don't think `struct` should respect the explicit aliased names, users should use `name_struct` instead. --- If your project is set up for it, you can repl

  1   2   >