Issues with Sort By on Table viz (for Presto)

[michellethomas]

There are a couple of issues with the way Sort By field is generating queries on Table charts for presto. Issues we have found so far have temporary workarounds. All issues are for table charts using the "AGGREGATE" query type.

1. If there is a metric alias that is the same as the column that is being aggregated, this results in an error for the generated presto query. This used to work and now does not. The workaround is to change the label for the metric. The query generated looks like:

SELECT cola, SUM(num) AS num
FROM table
GROUP BY cola
ORDER BY SUM(num) 

The presto error is: Invalid reference to output projection attribute from ORDER BY aggregation

2. There is no groupby in the generated sql query if there is no metric listed in the chart. I'm not sure if this was a change from previous behavior but this results in an error if the user tries to add a metric in the Sort By there is no GROUP BY in the query so it errors. Seems like we should be adding a groupby if there is no metric in the AGGREGATION section anyways(?)
3. If you are adding a Sort By metric where the column is also in the groupby, you then get an error that presto seems to have a problem with doing a MAX in the ORDER BY of an alias or column with the same name in the select SELECT "name" AS "name" or SELECT name with ORDER BY MAX(name) both result in errors. The generated presto query results in the error Invalid reference to output projection attribute from ORDER BY aggregation

Expected results

Charts should not error

Actual results

Charts show presto error

How to reproduce the bug

See above

Environment

(please complete the following information):

• superset version: master

Checklist

Make sure to follow these steps before submitting your issue - thank you!

• I have checked the superset logs for python stacktraces and included it here as text if there are any.
• I have reproduced the issue with at least the latest released version of superset.
• I have checked the issue tracker for the same issue and I haven't found one similar.

@junlincc @ktmud

[junlincc]

we touched this area recently as well i think? @ktmud let us know if the regression is not related to your change in table chart.