[Fix] Ensure inferred primary_key is a List[str] #10984
Merged
+95
−4
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #10983
Problem
Even though our schemas dictate that node.primary_key is a
List[str]
, it is possible forNone
values to creep into theprimary_key
ifunique
orunique_combination_of_columns
are not specified correctly and contain null values. This is a violation of our published manifest schema and can lead to parsing issues downstream.Solution
Do a better job validating the types of generic test kwargs primary key inference is depending on, especially given they may come from other packages like dbt_utils and not be validated at all (unlike our own constraint structures).
This is not a breaking behaviour change -- no project should have been relying on
null
values in theprimary_key
as they are semantically useless. If anything it was probably breaking parsing / validation on the consumption end if theunique
orunique_combination_of_columns
was invalid in niche cases.Checklist