[SEMANTIC-247] [Bug] Derived metrics with parenthesis in the expression cannot be calculated with metrics.develop macro
Closed this issue · 4 comments
Is this a new bug in dbt_metrics?
- I believe this is a new bug in dbt_metrics
- I have searched the existing issues, and I could not find an existing issue for this bug
Current Behavior
I have defined a derived metric (conversion_rate
) which has the following expression:
- expression: number_of_orders/(0.9*sessions)
Where number_of_orders (count_distinct of a column)
and sessions (sum of a column)
are two simple metrics.
Additionally, I have used metrics.develop
macro to calculate the value of conversion_rate
using the following macro:
select * from {{metrics.develop(develop_yml=my_metrics_yml, metric_list=['conversion_rate'], grain='month')}}
And I have obtained this error:
syntax error line 89 at position 90 unexpected 'as'
After debugging the dbt-metrics package, I have got to the line 22 of the dbt_metrics/macros/variables/get_metric_definition.sql file and obtained what follows:
{% set metric_expression = metric_definition.expression | replace("metric(","") | replace(")","") | replace("{{","") | replace("}}","") | replace("'","") | replace('"',"") %}
It seems that this part of the code is removing the last parenthesis of the expression of the metric conversion_rate
and causes the query to fail.
Expected Behavior
I would expect the metric could be calculated without any problem with metrics.develop
macro.
Steps To Reproduce
See above.
Relevant log output
No response
Environment
- dbt-adapter & version:v1.4.1
- dbt_metrics version:v1.4.1
Which database adapter are you using with dbt?
snowflake
Additional Context
I have temporarily adjusted the dbt-metrics package code by replacing the code line mentioned above by a more concrete replacement expression:
{% set metric_expression = metric_definition.expression | replace(" ","") | replace("{{metric('","") | replace("')}}","") | replace("'","") | replace('"',"") %}
And it has worked without any problem.
@josepfranquetf wow, great find and beautiful that you found the fix as well! Could you submit a PR with your fix? We'll be releasing the 1.5 branch late this week/early next week and I'd love to make sure this is included. Unfortunately I don't have the bandwidth to take it on but I'm happy to review for you!
Hi @callum-mcdata, many thanks for your answer! I have just opened a PR, which you can find here. Tell me in case you need something else!
This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.
Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest; add a comment to notify the maintainers.