[SPARK-23281][SQL] Query produces results in incorrect order when a composite order by clause refers to both original columns and aliases #20453

dilipbiswal · 2018-01-31T09:24:55Z

What changes were proposed in this pull request?

Here is the test snippet.

scala> Seq[(Integer, Integer)](
     |         (1, 1),
     |         (1, 3),
     |         (2, 3),
     |         (3, 3),
     |         (4, null),
     |         (5, null)
     |       ).toDF("key", "value").createOrReplaceTempView("src")

scala> sql(
     |         """
     |           |SELECT MAX(value) as value, key as col2
     |           |FROM src
     |           |GROUP BY key
     |           |ORDER BY value desc, key
     |         """.stripMargin).show
+-----+----+
|value|col2|
+-----+----+
|    3|   3|
|    3|   2|
|    3|   1|
| null|   5|
| null|   4|
+-----+----+
```SQL
Here is the explain output :

```SQL
== Parsed Logical Plan ==
'Sort ['value DESC NULLS LAST, 'key ASC NULLS FIRST], true
+- 'Aggregate ['key], ['MAX('value) AS value#9, 'key AS col2#10]
   +- 'UnresolvedRelation `src`

== Analyzed Logical Plan ==
value: int, col2: int
Project [value#9, col2#10]
+- Sort [value#9 DESC NULLS LAST, col2#10 DESC NULLS LAST], true
   +- Aggregate [key#5], [max(value#6) AS value#9, key#5 AS col2#10]
      +- SubqueryAlias src
         +- Project [_1#2 AS key#5, _2#3 AS value#6]
            +- LocalRelation [_1#2, _2#3]
``` SQL
The sort direction is being wrongly changed from ASC to DSC while resolving ```Sort``` in
resolveAggregateFunctions.

The above testcase models TPCDS-Q71 and thus we have the same issue in Q71 as well.

## How was this patch tested?
A few tests are added in SQLQuerySuite.

…ite order by clause refers to both original columns and aliases

SparkQA · 2018-01-31T12:47:58Z

Test build #86870 has finished for PR 20453 at commit 24e858c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dilipbiswal · 2018-01-31T19:33:50Z

cc @gatorsmile @cloud-fan

gatorsmile · 2018-01-31T21:32:57Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

@@ -1493,7 +1493,7 @@ class Analyzer(
          // to push down this ordering expression and can reference the original aggregate
          // expression instead.
          val needsPushDown = ArrayBuffer.empty[NamedExpression]
-          val evaluatedOrderings = resolvedAliasedOrdering.zip(sortOrder).map {
+          val evaluatedOrderings = resolvedAliasedOrdering.zip(unresolvedSortOrders).map {


A good catch! Is this a bug since 1.6.x?

@gatorsmile Yeah.. seems like it sean.

gatorsmile · 2018-01-31T21:52:23Z

LGTM

Thanks! Merged to master/2.3/2.2

dilipbiswal · 2018-01-31T21:53:04Z

Thanks a LOT @gatorsmile

…omposite order by clause refers to both original columns and aliases ## What changes were proposed in this pull request? Here is the test snippet. ``` SQL scala> Seq[(Integer, Integer)]( | (1, 1), | (1, 3), | (2, 3), | (3, 3), | (4, null), | (5, null) | ).toDF("key", "value").createOrReplaceTempView("src") scala> sql( | """ | |SELECT MAX(value) as value, key as col2 | |FROM src | |GROUP BY key | |ORDER BY value desc, key | """.stripMargin).show +-----+----+ |value|col2| +-----+----+ | 3| 3| | 3| 2| | 3| 1| | null| 5| | null| 4| +-----+----+ ```SQL Here is the explain output : ```SQL == Parsed Logical Plan == 'Sort ['value DESC NULLS LAST, 'key ASC NULLS FIRST], true +- 'Aggregate ['key], ['MAX('value) AS value#9, 'key AS col2#10] +- 'UnresolvedRelation `src` == Analyzed Logical Plan == value: int, col2: int Project [value#9, col2#10] +- Sort [value#9 DESC NULLS LAST, col2#10 DESC NULLS LAST], true +- Aggregate [key#5], [max(value#6) AS value#9, key#5 AS col2#10] +- SubqueryAlias src +- Project [_1#2 AS key#5, _2#3 AS value#6] +- LocalRelation [_1#2, _2#3] ``` SQL The sort direction is being wrongly changed from ASC to DSC while resolving ```Sort``` in resolveAggregateFunctions. The above testcase models TPCDS-Q71 and thus we have the same issue in Q71 as well. ## How was this patch tested? A few tests are added in SQLQuerySuite. Author: Dilip Biswal <dbiswal@us.ibm.com> Closes #20453 from dilipbiswal/local_spark.

gatorsmile · 2018-01-31T22:04:00Z

sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala

@@ -18,7 +18,6 @@
 package org.apache.spark.sql

 import java.io.File
-import java.math.MathContext


BTW, please do not remove the unnecessary import next time. Thanks!

When I backported it to 2.2, I hit a build break issue because it is still being used by the other test cases in 2.2

@gatorsmile Oh.. OK.. Sorry about it. I will not remove next time.

…omposite order by clause refers to both original columns and aliases ## What changes were proposed in this pull request? Here is the test snippet. ``` SQL scala> Seq[(Integer, Integer)]( | (1, 1), | (1, 3), | (2, 3), | (3, 3), | (4, null), | (5, null) | ).toDF("key", "value").createOrReplaceTempView("src") scala> sql( | """ | |SELECT MAX(value) as value, key as col2 | |FROM src | |GROUP BY key | |ORDER BY value desc, key | """.stripMargin).show +-----+----+ |value|col2| +-----+----+ | 3| 3| | 3| 2| | 3| 1| | null| 5| | null| 4| +-----+----+ ```SQL Here is the explain output : ```SQL == Parsed Logical Plan == 'Sort ['value DESC NULLS LAST, 'key ASC NULLS FIRST], true +- 'Aggregate ['key], ['MAX('value) AS value#9, 'key AS col2#10] +- 'UnresolvedRelation `src` == Analyzed Logical Plan == value: int, col2: int Project [value#9, col2#10] +- Sort [value#9 DESC NULLS LAST, col2#10 DESC NULLS LAST], true +- Aggregate [key#5], [max(value#6) AS value#9, key#5 AS col2#10] +- SubqueryAlias src +- Project [_1#2 AS key#5, _2#3 AS value#6] +- LocalRelation [_1#2, _2#3] ``` SQL The sort direction is being wrongly changed from ASC to DSC while resolving ```Sort``` in resolveAggregateFunctions. The above testcase models TPCDS-Q71 and thus we have the same issue in Q71 as well. ## How was this patch tested? A few tests are added in SQLQuerySuite. Author: Dilip Biswal <dbiswal@us.ibm.com> Closes apache#20453 from dilipbiswal/local_spark.

[SPARK-23281] Query produces results in incorrect order when a compos…

24e858c

…ite order by clause refers to both original columns and aliases

gatorsmile reviewed Jan 31, 2018

View reviewed changes

asfgit closed this in 9ff1d96 Jan 31, 2018

gatorsmile reviewed Jan 31, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-23281][SQL] Query produces results in incorrect order when a composite order by clause refers to both original columns and aliases #20453

[SPARK-23281][SQL] Query produces results in incorrect order when a composite order by clause refers to both original columns and aliases #20453

dilipbiswal commented Jan 31, 2018 •

edited

Loading

SparkQA commented Jan 31, 2018

dilipbiswal commented Jan 31, 2018

gatorsmile Jan 31, 2018

dilipbiswal Jan 31, 2018

gatorsmile commented Jan 31, 2018

dilipbiswal commented Jan 31, 2018

gatorsmile Jan 31, 2018

dilipbiswal Jan 31, 2018

[SPARK-23281][SQL] Query produces results in incorrect order when a composite order by clause refers to both original columns and aliases #20453

[SPARK-23281][SQL] Query produces results in incorrect order when a composite order by clause refers to both original columns and aliases #20453

Conversation

dilipbiswal commented Jan 31, 2018 • edited Loading

What changes were proposed in this pull request?

SparkQA commented Jan 31, 2018

dilipbiswal commented Jan 31, 2018

gatorsmile Jan 31, 2018

Choose a reason for hiding this comment

dilipbiswal Jan 31, 2018

Choose a reason for hiding this comment

gatorsmile commented Jan 31, 2018

dilipbiswal commented Jan 31, 2018

gatorsmile Jan 31, 2018

Choose a reason for hiding this comment

dilipbiswal Jan 31, 2018

Choose a reason for hiding this comment

dilipbiswal commented Jan 31, 2018 •

edited

Loading