You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/03/10 15:57:51 UTC

[GitHub] [superset] diegomedina248 opened a new pull request #19102: fix: string aggregation is incorrect in PivotTableV2

diegomedina248 opened a new pull request #19102:
URL: https://github.com/apache/superset/pull/19102


   ### SUMMARY
   The aggregation functions for PivotTableV2 only take into account numeric values.
   This PR adds support for strings, only for the aggregation functions that were supported on PivotTable originally.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   https://user-images.githubusercontent.com/17252075/157701090-70ad6668-b319-485a-82cb-7edabcc042c6.mov
   
   ### TESTING INSTRUCTIONS
   Follow the example described in the issue
   
   ### ADDITIONAL INFORMATION
   Fixes #18572 
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


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

[GitHub] [superset] diegomedina248 commented on pull request #19102: fix: string aggregation is incorrect in PivotTableV2

Posted by GitBox <gi...@apache.org>.
diegomedina248 commented on pull request #19102:
URL: https://github.com/apache/superset/pull/19102#issuecomment-1069115936


   > Just noticed 1 issue - selecting `Median` results in returning nulls <img alt="image" width="1790" src="https://user-images.githubusercontent.com/15073128/158579445-fcd782d9-58ee-4f86-bf03-3e705807d5ef.png">
   > 
   > Also, selecting `List unique values` or `Sum as fraction of total` returns NaNs
   
   @kgabryje I only made the changes for the ones present in v1. Can make it for all options if you're ok with it.


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

[GitHub] [superset] kgabryje commented on a change in pull request #19102: fix: string aggregation is incorrect in PivotTableV2

Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #19102:
URL: https://github.com/apache/superset/pull/19102#discussion_r827890007



##########
File path: superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.js
##########
@@ -353,7 +370,7 @@ const baseAggregatorTemplates = {
                 throw new Error('unknown mode for runningStat');
             }
           },
-          format: formatter,
+          format: x => (typeof x === 'string' ? x : formatter(x)),

Review comment:
       Nit: since we do the same operation in line 241, could you extract that logic to a function?




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