rusackas commented on PR #42040: URL: https://github.com/apache/superset/pull/42040#issuecomment-5012322819
Thanks @rdg0, the `adhoc_column_to_sqla` delegation for the `is_adhoc_column` branch is the right fix here and looks good, matching the `need_groupby` path and the regression test covers it. One thing I'm stuck on: the `columns_by_name.get(_column_label)` lookup a few lines below it, in the plain-string branch. `_column_label` is just the original `selected` value, and anything in `columns_by_name` already `continue`d out via the guard above (the SC-111745 one). So `target_col` can never resolve there and we always fall to `literal_column`. Is that intentional, or leftover from swapping out the old `quoted_columns_by_name` check? If it's dead, might as well simplify that branch down to just the `literal_column` case. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
