Skip to content
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

Avoid false positives in date_spine macro #211

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dbeatty10
Copy link
Contributor

@dbeatty10 dbeatty10 commented May 14, 2024

resolves #210
resolves #208

Problem

While working on dbt-labs/dbt-core#10075, I noticed a few things about this test case:

  1. The test can give a false positives
  2. The date range is larger than needed
  3. There are hard-coded adapter types

The first one is the most impactful. This PR solves the first two but leaves the last one unsolved.

Solution

  1. Use a full outer join rather than a left join to eliminate possibility of a false positive
  2. Reduce the date range from 9 down to 3 (which gets rid of 36 lines of code)

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development, and it appears to resolve the stated issue
  • New tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc.)

Copy link

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 11, 2024
@adrianburusdbt
Copy link
Contributor

Hey @dbeatty10, is this PR still needed? It's in draft for a long time now.
If yes, a couple of questions:

  1. could you please describe an example when the false positive can be reproduced?
  2. what is the reason the current range needs to be changed (aside from the added code lines)?

Thanks!

@github-actions github-actions bot removed the Stale label Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants