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

Add a new macro to test numeric types with specific precision and scale #263

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

samousavih
Copy link

@samousavih samousavih commented Jun 5, 2023

Issue this PR Addresses/Closes

Closes #(262)

If you don't have an issue #, please first open an issue on the repo before submitting a PR to discuss the changes you'd like to make.

Summary of Changes

  1. A new macro to assert numeric types and their scale and precision

Why Do We Need These Changes

Current macro to assert column types doesn't include scale and precision.

Reviewers

@clausherther

@clausherther
Copy link
Contributor

clausherther commented Jun 10, 2023

Hi, sorry for the delayed response! Instead of making a new test, would it be possible to make this an optional path in expect_column_values_to_be_of_type?

(FYI, BigQuery fails on this test.)

@samousavih
Copy link
Author

Hey @clausherthe, thanks for the reply,
One option is to make 'expect_column_values_to_be_of_type' and 'expect_column_values_to_be_in_type_list' assert precision and scale when mentioned and ignore them when not, which is potentially what most would expect from the test.

- dbt_expectations.expect_column_values_to_be_of_type:
      column_type: numeric(2,1)
- dbt_expectations.expect_column_values_to_be_in_type_list:
      column_type_list: [numeric(2,1)]
- dbt_expectations.expect_column_values_to_be_of_type:
      column_type: numeric
- dbt_expectations.expect_column_values_to_be_in_type_list:
      column_type_list: [numeric]

How does this look like to you?
Only minor issue might be it could fail existing tests once package users upgrade, which could be a good thing depending how you look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants