[GitHub] villebro commented on issue #6831: Do label name mutation before anything else on the dataframe

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

villebro commented on issue #6831: Do label name mutation before anything else on the dataframe
URL: https://github.com/apache/incubator-superset/pull/6831#issuecomment-461710880
 
 
   I will test for regressions during weekend. One thing I did note is that the new dataframe mutation logic has similarities with the existing label mutation logic. The original one serves two purposes:
   1. Making label names compatible with the db engine. Example: making sure that BigQuery labels don't start with a number, making sure the label name doesn't exceed the max length etc.
   2. Making sure that the dataframe label name is exactly the same as is expected. Example: Redshift changes `SUM(x)` to `sum(x)`.
   
   The new method essentially takes care of the latter case, making that task redundant from the original label mutator. Going forward having this overlap will be very confusing for developers, and would be good to refactor, essentially deferring case 2 mutation the the dataframe mutator only. Once the dust settles I can put in a PR to clarify that.

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


With regards,
Apache Git Services

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