Issue 9656 Raise an error when check_cols has duplicated values

[ariosramirez]

resolves #9656

Problem

Currently, the check strategy for Snapshots does not throw an error if there are duplicate column names specified in the check_cols config, and accidentally specifying a column twice results in a record being inserted even if none of the column values changed.

Problem described in #9656 (comment)

Solution

This PR adds validation to detect duplicate column names in the check_cols configuration of a snapshot. It raises an error if duplicates are found.

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
• This PR includes tests, or 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) or this PR has already received feedback and approval from Product or DX
• This PR includes type annotations for new and modified functions

[github-actions]

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

[github-actions]

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

[jordivandooren]

Even though .get() is used, if check_cols is missing, this will result in an error because NoneType has no len().

Suggested change

	if len(data.get("check_cols")) != len(set(data.get("check_cols"))):
	if "check_cols" in data and len(data["check_cols"]) != len(set(data["check_cols"])):

But, judging by the code above it, this check should only apply in case of the check strategy, where the existence of this check_cols has been validated beforehand. I suggest moving this new validation to after line 40.

I expect the current implementation to break the breaks the timestamp strategy, because of the absence of check_cols.

[ariosramirez]

@jordivandooren Could you please review the latest changes? I updated my code, and everything is now up-to-date. The duplicated columns validation has been moved inside the final_validate function.

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: ariosramirez.
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@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: ariosramirez.
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@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: ariosramirez.
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@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: ariosramirez.
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@example.com
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.99%. Comparing base (c6b8f7e) to head (f6a7335).
Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9698      +/-   ##
==========================================
+ Coverage   88.90%   88.99%   +0.09%     
==========================================
  Files         180      181       +1     
  Lines       22856    22974     +118     
==========================================
+ Hits        20319    20445     +126     
+ Misses       2537     2529       -8     

Flag	Coverage Δ
integration	86.16% <66.66%> (+0.06%)	⬆️
unit	62.39% <100.00%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
Unit Tests	62.39% <100.00%> (-0.03%)	⬇️
Integration Tests	86.16% <66.66%> (+0.06%)	⬆️

[dataders]

• check if your git client is configured with an email to sign commits git config --list | grep email

• If not, set it up using git config --global user.email email@example.com

• Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@ariosramirez did you do these steps already as well?

[QMalcolm]

Hi @ariosramirez, I think I understand what the problem is.

I checked our CLA database, and it does look like you've signed it. What I believe is happening is that cla-bot checks every commit in the PR. A majority of your commits are signed with the email ariosramirez.data@gmail.com, and that email address seems correct. However, four commits (8082fa7, 5343684, 67d5cb4, and 7bd6cb5) are signed with the email andres@beek.io. It appears that email address is no longer associated with your github account. If you still have access to that email address, I recommend re-adding it to your list of associated email address in github (you can have multiple, I myself have five email address associated with my github account). If you no longer have access to that email address, you may need to modify the signature on those commits.

[QMalcolm]

@ariosramirez Alternatively, you can squash the problematic (or all the commits) into one new commit. This should give all the work your current email, I believe

[ariosramirez]

QMalcolm

Thanks, @QMalcolm the squash commit worked well.