[GitHub] [superset] zhaoyongjie commented on a diff in pull request #19842: fix: count(distinct column_name) in metrics

[GitBox <gi...@apache.org>]

zhaoyongjie commented on code in PR #19842:
URL: https://github.com/apache/superset/pull/19842#discussion_r858332944


##########
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetric.js:
##########
@@ -86,20 +89,30 @@ export default class AdhocMetric {
   }
 
   getDefaultLabel() {
-    const label = this.translateToSql(true);
+    const label = this.translateToSql({ useVerboseName: true });
     return label.length < 43 ? label : `${label.substring(0, 40)}...`;
   }
 
-  translateToSql(useVerboseName = false) {
+  translateToSql(
+    params = { useVerboseName: false, transformCountDistinct: false },
+  ) {
     if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
       const aggregate = this.aggregate || '';
       // eslint-disable-next-line camelcase
       const column =
-        useVerboseName && this.column?.verbose_name
+        params.useVerboseName && this.column?.verbose_name
           ? `(${this.column.verbose_name})`
           : this.column?.column_name
           ? `(${this.column.column_name})`
           : '';
+      // transform from `count_distinct(column)` to `count(distinct column)`
+      if (
+        params.transformCountDistinct &&
+        aggregate === AGGREGATES.COUNT_DISTINCT &&
+        /^\(.*\)$/.test(column)
+      ) {
+        return `COUNT(DISTINCT ${column.slice(1, -1)})`;
+      }

Review Comment:
   The previous logic may not have been covered by UT, so I skipped these very carefully. I think it would be better to add some UTs to this part of the code and then modify it all together. 
   
   Of course, if this needs to be done in this PR, I'm all for it. What do you think about this?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org