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

feat(nulltest): add not_null option to unique_combination_of_columns #963

Closed
halfos opened this issue Nov 13, 2024 · 3 comments
Closed

feat(nulltest): add not_null option to unique_combination_of_columns #963

halfos opened this issue Nov 13, 2024 · 3 comments
Labels
enhancement New feature or request triage

Comments

@halfos
Copy link

halfos commented Nov 13, 2024

Problem

We perform unique and not null tests on all primary key columns. Previously, not_null data tests had to be specified repeatedly for the same columns that are also listed in the unique_combination_of_columns test.

Solution

The proposed solution adds an optional for loop that performs not null tests on each column in the unique_combination_of_columns test. Gives neater code. The default parameter is false, so it should not be a breaking change.

Describe the feature

A neat way to enforce primary keys is to have an option to not only use the "unique_combination_of_columns" test for unique tests of composite keys, but also for enforcing the not_null constraint for each of the underlying columns. This way developers have the option of not having to individually repeat single not_null tests for each of the columns listed in the "unique_combination_of_columns" test. This results in better readability, and hence better code quality.

Describe alternatives you've considered

Have considered writing a separate macro which combines these two types of tests, but I think it seemed more useful to have the not_null option inside the unique_combination_of_columns macro.

Additional context

Primary keys are not enforced on Snowflake, but unique_combination_of_columns and not_null can be used as a way around to enforce it.

Who will this benefit?

Enforcing primary keys in all databases.

Are you interested in contributing this feature?

Yes, already have a pull request ready (simple for loop).

@halfos
Copy link
Author

halfos commented Nov 13, 2024

Realized now that putting them together in one test may make it difficult to see which part of the test failed

@halfos
Copy link
Author

halfos commented Nov 13, 2024

A major flaw with this suggestion as it is coded right now is that when the test fails, it does not say whether it is the unique combination or not_null part that fails.

@halfos
Copy link
Author

halfos commented Nov 13, 2024

Might be better to concatenate composite keys into a single primary key column, and use a simple unique and not_null test separately as hinted in the docs: https://docs.getdbt.com/reference/resource-properties/constraints

@halfos halfos closed this as completed Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant