-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added ability to use expression_is_true as a column test #226
Added ability to use expression_is_true as a column test #226
Conversation
Oh this is really cool! As I just mentioned on your other PR, I'm going to do a big review on Friday! I have a related thought bouncing around in my head: What if we renamed the
Then you could call it like: version: 2
models:
- name: orders_snapshot
columns:
- name:
tests:
- assert_having_clause: "sum(case when is_current_version then 1 end) = 1" I could imagine it being useful in other spots, e.g. does a ledger of transactions result in a positive entry: version: 2
models:
- name: transactions
columns:
- name: order_id
tests:
- assert_having_clause: "sum(amount) > 0" Anyway, this is just a side-thought, and definitely not required for this PR at all, but wanted to chat through the thought with someone who is also thinking about this! |
LOVE the assert_having_clause idea! I can think of a few places we'd use it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rebase this from master, and also implement the changes I suggested (perhaps with better formatting :) than what I quickly wrote)
@@ -12,7 +13,7 @@ validation_errors as ( | |||
select | |||
* | |||
from meet_condition | |||
where not({{expression}}) | |||
where not( {{ expression if column_name is none else [column_name, expression] | join(' ') }}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this is a little hard to read for me, I think I'd prefer an if
statement:
where not( {{ expression if column_name is none else [column_name, expression] | join(' ') }}) | |
{% if column_name is none %} | |
where not({{ expression }}) | |
{% else %} | |
where not({{ column_name }} {{ expression }}) | |
{% endif %} |
I was just looking for something like this today. What do we need to do to get this one over the finish line? |
Reopened as #313 so I could make changes |
Description & motivation
I love the expression_is_true schema test, but often it feels like the tests I use them for should be a child of the column I'm testing against, instead of the whole relationship. So I modified the expression_is_true test to do exactly that.
Left the original tests as is (no regressions), and added 2 more tests to test with condition set and without it.
Checklist