SEPURI-SAI-KRISHNA commented on PR #45431: URL: https://github.com/apache/superset/pull/45431#issuecomment-6248971485
Both of these are right, thanks. Pushed a follow-up. **SIMPLE adhoc metrics.** `SqlaTable.adhoc_metric_to_sqla` resolves a SIMPLE metric's column against `columns_by_name` and calls `table_column.get_sqla_col(template_processor=...)`, and `TableColumn.get_sqla_col` renders the expression through the processor, so a calculated column's macro does reach the SQL with no `sqlExpression` anywhere in the metric. `metric_expression` now falls back to that column's expression, which covers `metrics`, `orderby` and `series_limit_metric` in one place. The `orderby` loop was reading `sqlExpression` off a dict directly, so it also goes through `metric_expression`/`column_expression` now rather than duplicating the lookup. **Filters on a metric name.** Same conclusion: a filter column that is not a dataset column is resolved against `metrics_by_name` and rendered via `SqlMetric.get_sqla_col`. One step before that, the chain also resolves the column against the columns' verbose names, which the previous lookup missed too since it only keyed on `column_name`, so filters now have their own resolver covering adhoc column, calculated column name, verbose name and saved metric. Running the same cases against the previous commit and this one, five flip: ``` orderby SIMPLE metric on a jinja calculated column False -> True series_limit_metric SIMPLE metric, same column False -> True metrics SIMPLE metric, same column False -> True filter on a calculated column's verbose name False -> True filter on a saved metric carrying jinja False -> True ``` The negative controls stay negative: the same shapes over a plain column, a plain saved metric and a plain column name all still return False, so no extra rendering. Test grew from 8 to 19 cases and the dataset now carries calculated columns and saved metrics so the name-based paths are exercised. -- 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]
