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

[CT-2774] Optional type aliasing for model contract data_type #8007

Closed
Tracked by #7979
jtcohen6 opened this issue Jun 30, 2023 · 3 comments · Fixed by #8592
Closed
Tracked by #7979

[CT-2774] Optional type aliasing for model contract data_type #8007

jtcohen6 opened this issue Jun 30, 2023 · 3 comments · Fixed by #8592
Assignees

Comments

@jtcohen6
Copy link
Contributor

Give users a way to opt into dbt adapters' built-in type translation, for the data_type defined in their contract.

config:
  contract:
    enforced: true
    alias_types: true

E.g. on Postgres/Redshift, I could write string instead of text and it would "just work". It would continue to "just work" if I take the same model with the same contract and run it on Snowflake/BigQuery/etc — if the type aliasing for those adapters is correctly implemented._ That's a big "if", which is why this functionality should be optional and off by default.

models:
  - name: my_model
    config:
      contract:
        enforced: true
        alias_types: true
    columns:
      - name: my_string_col
        data_type: string  # will be aliased to 'text' or kept as 'string' depending on data platform
      - name: my_int_col
        data_type: integer  # 'int' or 'int64'

The implementation should be fairly simple: Wherever we access the data_type for contract enforcement / DDL templating, we should wrap it in {{ return(api.Column.translate_type("string")) }}.

https://github.com/dbt-labs/dbt-core/blob/5d937802f1d5b34baeb8412cca68bfb33112a763/core/dbt/adapters/base/column.py#L23C1-L25

This would have the same effect as letting users write {{ dbt.type_timestamp() }} instead of just the string "timestamp", given those macros actually call translate_type behind the scenes.

@jtcohen6 jtcohen6 added enhancement New feature or request model_contracts labels Jun 30, 2023
@github-actions github-actions bot changed the title Optional type aliasing for model contract data_type [CT-2774] Optional type aliasing for model contract data_type Jun 30, 2023
@MichelleArk
Copy link
Contributor

That's a big "if", which is why this functionality should be optional and off by default.

It might actually be sensible for this to be the default behaviour -- given that translate_method is a no-op (returns its input) if there is no mapping available. This no-op behaviour looks consistent / not-overwritten by dbt labs maintained adapter implementations (bq, snowflake, redshift, spark)

@jtcohen6
Copy link
Contributor Author

@MichelleArk I'm open to giving that a go! We could try to make this change in an early v1.7 beta, and see if there are any adverse or surprising effects during prerelease testing. We might still want to give users a way to opt out.

@graciegoheen
Copy link
Contributor

Note from estimation meetings: We will want to add a test case in the adapter repos, even though all of the code changes will likely be in core

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

Successfully merging a pull request may close this issue.

5 participants