[GitHub] [spark] Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort behaviour

2019-09-12 Thread GitBox
Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort 
behaviour
URL: https://github.com/apache/spark/pull/25728#issuecomment-531099338
 
 
   I understood the same @ueshin since he said to match presto


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort behaviour

2019-09-12 Thread GitBox
Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort 
behaviour
URL: https://github.com/apache/spark/pull/25728#issuecomment-530767308
 
 
   Ok I understand. Also I think that might be a different PR? and then have 
different signatures for array_sort. For example:
   
   array_sort(column) -> asc by default
   array_sort(column, order)
   array_sort(column, comparatorFunction)
   
   Or you just want to have array_sort(column, comparatorFunction)? 
   
   I think having array_sort(column, order) is easier to use, and most of the 
times you just want to order asc or desc. At the same time having the 
comparatorFunction can open more possibilities. 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort behaviour

2019-09-10 Thread GitBox
Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort 
behaviour
URL: https://github.com/apache/spark/pull/25728#issuecomment-530207202
 
 
   How does it translate for this PR @ueshin ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort behaviour

2019-09-10 Thread GitBox
Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort 
behaviour
URL: https://github.com/apache/spark/pull/25728#issuecomment-530031571
 
 
   I think it would be good to have array_sort to be able to order in both 
ways, all the array function from 2.4.0 are "array_something" and it's kind of 
confusing to have sort_array.
   I think it would be great to have that unified.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort behaviour

2019-09-10 Thread GitBox
Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort 
behaviour
URL: https://github.com/apache/spark/pull/25728#issuecomment-529994076
 
 
   > I see, is it that the ordering of nulls is different?
   > I think we'd at least want the doc of each to comment on the difference 
with the other very similar method, to make this clear. I suppose one day we 
may add control over null ordering and deprecate one in favor of the other.
   
   Okay @srowen , I'll explain the ordering of nulls for all as they are now:
   `sort_array` in `ascending order` nulls are at the `beginning`
   `sort_array` in `descending order` nulls are at the `end`
   `array_sort` in `ascending order` nulls are at the `end`
   
   And after the PR:
   `sort_array` will remain the same
   `array_sort` in `ascending order` nulls are at the `end`
   `array_sort` in `descending order` nulls are at the `end`
   
   So this way the actual behaviour of `array_sort` remains the same.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort behaviour

2019-09-10 Thread GitBox
Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort 
behaviour
URL: https://github.com/apache/spark/pull/25728#issuecomment-529977320
 
 
   > @ueshin yeah I agree, the bigger question is, why do we have both 
`array_sort` and `sort_array`?
   > 
   > @kiszk looks like you added `array_sort` to match Presto in 
https://issues.apache.org/jira/browse/SPARK-23921 . Presto supports a 
comparator function rather than an ascending flag.
   > 
   > If `sort_array` does what we want already, and is the older method here, 
why not make `array_sort` an alias for it? We could deprecate one, but which 
one? `sort_array` has been around longer, but `array_sort` is at least the name 
in one other system.
   > 
   > Overall I'd just like to avoid duplicating the implementation, at least, 
even if both are supported as the function name.
   
   @srowen we couldn't make an alias between sort_array and array_sort because 
they don't have the same null policy, it would change the behaviour of 
array_sort.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort behaviour

2019-09-10 Thread GitBox
Gschiavon commented on issue #25728: [SPARK-29020][SQL] Improving array_sort 
behaviour
URL: https://github.com/apache/spark/pull/25728#issuecomment-529976886
 
 
   @srowen we couldn't make an alias between `sort_array` and `array_sort` 
because they don't have the same null policy, it would change the behaviour of 
`array_sort`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org