adamcunnington-mlg commented on PR #32228:
URL: https://github.com/apache/superset/pull/32228#issuecomment-2671926712

   Hi @betodealmeida @Vitor-Avila @eschutho we will send a separate note about 
this but it looks like the requirement was interpreted too explicitly. I'm 
sorry that we did not give more explicit examples. We are testing today and see 
that the solution implemented is one that recursively expands only metric 
references within the recursive expansion.
   
   Rather, we need the jinja to be expanded such that metric references are 
fully expanded.
   
   E.g. metric a depends on metric b and metric b contains jinja other than 
just a metric macro that needs to be expanded.
   
   We assumed this would be covered by "recursively expand metric macro" but 
appreciate how this could be understood too narrowly.
   
   An example of the problem:
   ```
   SAFE_DIVIDE(
     {{ 
metric("metric_mrt_all_clients__cross_channel__summary__dynamic_kpi_count") }},
     SUM(`engagements`)
   )
   ```
   
   gets expanded to a query like this
   ```
   -- 6dcd92a04feb50f14bbcf07c661680ba
   SELECT SAFE_DIVIDE(
     SUM(0{% for x in filter_values("dynamic_kpi") %}
       {{ " + COALESCE(kpis." + x + "_count, 0)" }}
     {% endfor %}),
     SUM(`engagements`)
   ) AS `metric_mrt_all_clients__cross_channel__summary__dynamic_cvr` 
   FROM `DBT_ANDY`.`CROSS_CHANNEL` ORDER BY 
`metric_mrt_all_clients__cross_channel__summary__dynamic_cvr` DESC
    LIMIT 1000
   -- 6dcd92a04feb50f14bbcf07c661680ba;
   ```
   
   Here you can see 
`metric_mrt_all_clients__cross_channel__summary__dynamic_kpi_count`'s 
expression contained jinja that has not been expanded and so bad syntax gets 
sent to DB.


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

Reply via email to