-
Notifications
You must be signed in to change notification settings - Fork 40
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 equals macro that handles null value comparison #383
Conversation
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @adrianburusdbt |
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @adrianburusdbt |
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @adrianburusdbt |
dbt/include/global_project/macros/materializations/models/incremental/merge.sql
Outdated
Show resolved
Hide resolved
dbt/include/global_project/macros/materializations/models/incremental/merge.sql
Outdated
Show resolved
Hide resolved
dbt/include/global_project/macros/materializations/snapshots/helpers.sql
Outdated
Show resolved
Hide resolved
dbt/include/global_project/macros/materializations/snapshots/helpers.sql
Outdated
Show resolved
Hide resolved
dbt/include/global_project/macros/materializations/snapshots/snapshot_merge.sql
Outdated
Show resolved
Hide resolved
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.
thanks @mikealfare for doing the bulk of CR here.
This reverts commit 048cb2b.
resolves #
Problem
Null comparison was not handled properly in the base adapter. While some value comparisons were missing this case altogether, others were fixed but not in a repeatable, organized way.
Solution
Based loosely on #110, but ultimately it's following the suggestions made here: dbt-labs/dbt-core#6997 (comment)
Extracted the logic that was in the test fixture into an 'equals.sql' macro that can be reused and replaced all other usages with this utility macro.
Checklist