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

Allow to use a costum field as check-cols updated_at #3376

Merged

Conversation

PJGaetan
Copy link
Contributor

resolves #1844

Description

Allows the use of updated_at field while using check strategy in a snapshot.
The timestamp used to update dbt_valid_from will come from the costum field instead of the default current timestamp.

I have added a test to ensure that rows are invalidated with the right timestamp. Not sure it is in the right place to add the test but I didn't know how to pick up relation_b otherwise.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot
Copy link

cla-bot bot commented May 20, 2021

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: PierreJustin Gaetan.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https:/settings/emails

@PJGaetan PJGaetan force-pushed the feature/snapshot-check-cols-updated-at branch from 987835f to 66e0696 Compare May 20, 2021 05:37
@cla-bot
Copy link

cla-bot bot commented May 20, 2021

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: PierreJustin Gaetan.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https:/settings/emails

@PJGaetan PJGaetan force-pushed the feature/snapshot-check-cols-updated-at branch from 66e0696 to c33c253 Compare May 20, 2021 05:54
@cla-bot cla-bot bot added the cla:yes label May 20, 2021
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PJGaetan This looks great to me! Nice work on the test.

I left a small suggestion on the changelog. Could you also:

  • Pull in develop
  • Move your changelog note up to v0.20.0 (rather than v0.20.0b1)
  • Add yourself to the list of Contributors

CHANGELOG.md Outdated
@@ -41,6 +41,7 @@ Contributors:
- Add native support for Postgres index creation ([#804](https:/fishtown-analytics/dbt/issues/804), [3106](https:/fishtown-analytics/dbt/pull/3106))
- Less greedy test selection: expand to select unselected tests if and only if all parents are selected ([#2891](https:/fishtown-analytics/dbt/issues/2891), [#3235](https:/fishtown-analytics/dbt/pull/3235))
- Prevent locks in Redshift during full refresh in incremental materialization. ([#2426](https:/fishtown-analytics/dbt/issues/2426), [#2998](https:/fishtown-analytics/dbt/pull/2998))
Add optional `updated_at` config parameter to invalidate rows with the inputed column timestamp in snapshot using `check` strategy. ([#1844](https:/fishtown-analytics/dbt/issues/1844), [#3376](https:/fishtown-analytics/dbt/pull/3376))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small suggestion on the change wording, to make it clear what the current/default behavior is:

Suggested change
Add optional `updated_at` config parameter to invalidate rows with the inputed column timestamp in snapshot using `check` strategy. ([#1844](https:/fishtown-analytics/dbt/issues/1844), [#3376](https:/fishtown-analytics/dbt/pull/3376))
- Support optional `updated_at` config parameter with `check` strategy snapshots. If not supplied, will use current timestamp (default). ([#1844](https:/fishtown-analytics/dbt/issues/1844), [#3376](https:/fishtown-analytics/dbt/pull/3376))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely fells better this way

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @PJGaetan!

@jtcohen6 jtcohen6 merged commit 7418f36 into dbt-labs:develop May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants