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 multiple partition columns in mutually_exclusive_ranges.sql without concatenation #938

Closed
Caitlin-Syntax opened this issue Jul 20, 2024 · 2 comments
Labels
enhancement New feature or request Stale triage

Comments

@Caitlin-Syntax
Copy link

Describe the feature

An alternative title for this request could be "Update or eliminate column selection that reduces test functionality."

The mutually_exclusive_ranges test currently only allows partitioning by one column at a time unless you are "sneaky" and concatenate values together in the partition_by config, but this is not well-documented in the README, and concatenation introduces failure risk if not done carefully (this is likely negligible risk, but it's non-zero).

There are use plenty of use cases where it is meaningful to partition by more than one column. For example, in the US healthcare industry, a patient may have two active insurance plans at one time, but not two active plans from the same insurance company, so to create a table listing the active time ranges for these coverages and test that they do not overlap, you need to be able to partition by both patient and insurance company. Or as seen in Issue 423, you may need to break up customer account records across projects. (This issue is actually introduced by the fix for that issue because prior to the column alias being added, a comma delimited list of columns would work as desired, so I'm not sure if there's a reason that wasn't the recommended solution.)

This can be achieved with the current test by eliminating or making a small adjustment to the line in the window_functions CTE that effectively forces a single partition column today: {{ partition_by }} as partition_by_col. The partition_by_col value is not used anywhere in downstream logic, so it could either be deleted entirely, or it could be updated to wrap the {{ partition_by }} value in quotes. Either option would allow a comma-delimited list or a concatenated list to work as a partition_by config statement.

At minimum, the README could be updated to indicate that the concatenation technique is an available option.

Describe alternatives you've considered

  • Use the concatenation option
  • Duplicate this test locally and remove the line as described above

Additional context

No major context to consider as far as I'm aware. This adjustment should be agnostic of any particular database/platform unless there's some sort of problem with the "wrap it in quotes" option in a db I'm unfamiliar with.

Who will this benefit?

Anyone who needs to partition by more than one column! Examples provided above.

Are you interested in contributing this feature?

Happy to help, but it would probably take more time and effort to get me set up than for someone "plugged in" already to simply make the fix.

@Caitlin-Syntax Caitlin-Syntax added enhancement New feature or request triage labels Jul 20, 2024
Copy link

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

@github-actions github-actions bot added the Stale label Jan 17, 2025
Copy link

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale triage
Projects
None yet
Development

No branches or pull requests

1 participant