Cartesian Join Update

[sisu-callum]

Description

Based on this issue, I've added a parameter called cartesian_join that can be set to true or false¹. The default value for this is true and it uses the current logic. If the parameter is set to false, it uses the logic raised in the issue and excludes impossible combinations of values.

Changes

• get_metric_sql: This macro now has an additional if statement in the spine_values section. I'm unsure of what dbt_labs best practice is but I figured having the else statement correspond to the current logic was the safest practice². Only if the cartesian_join parameter is set to false will it apply the other logic.
• metric: Added the cartesian_join field and set default to true.

Tests

I'll be completely honest, I was not entirely sure how to test this outside of my repo with DWH connection. I am sure there is some way that all the package maintainers are aware of but I didn't want to bother @fivetran-joemarkiewicz over the weekend. So I copied over the logic into my repo and tested it there. It worked fine for me! 🤞 it passes all the other tests.

Footnotes

Footnotes

1. To be honest, I'm not married to that name? I kept it 1:1 with the language used in the filed issue but I think there is probably a better way to name it for the average user. Maybe exclude_impossible_combinations and set the default value to false. ↩

2. See the beginning of the if statement current set to == false, which is not default ↩

[callum-mcdata]

As the creator of this PR, I'm going to close it out and re-create one using best practices. Namely I won't be adding the parameter in the macro that allows for the cartesian join. I'll just be removing it outright