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-3181] [Bug] ConstraintType.custom doesn't mix with adapter CONSTRAINT_SUPPORT #90

Closed
2 tasks done
jtcohen6 opened this issue Oct 4, 2023 · 2 comments · Fixed by #91
Closed
2 tasks done
Assignees
Labels
bug Something isn't working Medium Severity bug with minor impact that does not have resolution timeframe requirement

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Oct 4, 2023

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

ConstraintType.custom is missing from CONSTRAINT_SUPPORT, leading to a KeyError during process_parsed_constraint.

We could fix this in either method, but we'll need to account for adapter-level overrides.

https://github.com/dbt-labs/dbt-core/blob/main/core/dbt/adapters/base/impl.py#L226-L232

https://github.com/dbt-labs/dbt-core/blob/eac13e3bd38b7f29401444ca77ee243ec4d8f29d/core/dbt/adapters/base/impl.py#L1441-L1462

Expected Behavior

There should not be a KeyError.

Custom constraints should be ConstraintSupport.NOT_ENFORCED by default. In the future, we could maybe let users tell us if a constraint is enforced or not (i.e. whether removing that constraint constitutes a breaking change).

Steps To Reproduce

It's possible to use custom constraints for any attributes that need application within the column spec of the create table DDL. For example, custom compression values on columns in Redshift.

  1. Create a model:
-- models/my_model.sql
select 1 as a_number
  1. Define a contract for that model:
models:

  - name: my_model
    config:
      materialized: table
      contract:
        enforced: true

    columns:
      - name: a_number
        data_type: integer
        constraints:
          - type: custom
            expression: "encode bytedict"
            warn_unenforced: False
            warn_unsupported: False

Relevant log output

File "/Users/jerco/dev/product/dbt-core/core/dbt/adapters/base/impl.py", line 1434, in render_raw_columns_constraints
    c = cls.process_parsed_constraint(constraint, cls.render_column_constraint)
  File "/Users/jerco/dev/product/dbt-core/core/dbt/adapters/base/impl.py", line 1459, in process_parsed_constraint
    if cls.CONSTRAINT_SUPPORT[parsed_constraint.type] != ConstraintSupport.NOT_SUPPORTED:
KeyError: <ConstraintType.custom: 'custom'>

Environment

- OS: macOS 13.4.1
- Python: 3.10.11
- dbt: 1.7.0-b2 (main)

Which database adapter are you using with dbt?

No response

Additional Context

No response

@jtcohen6 jtcohen6 added the bug Something isn't working label Oct 4, 2023
@github-actions github-actions bot changed the title [Bug] ConstraintType.custom doesn't mix with adapter CONSTRAINT_SUPPORT [CT-3181] [Bug] ConstraintType.custom doesn't mix with adapter CONSTRAINT_SUPPORT Oct 4, 2023
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Oct 4, 2023

Fix that I implemented locally:

    @classmethod
    def process_parsed_constraint(
        cls, parsed_constraint: Union[ColumnLevelConstraint, ModelLevelConstraint], render_func
    ) -> Optional[str]:
        # skip checking enforcement if this is a 'custom' constraint
        if parsed_constraint.type == ConstraintType.custom:
            return render_func(parsed_constraint)

@jtcohen6 jtcohen6 added the Medium Severity bug with minor impact that does not have resolution timeframe requirement label Oct 24, 2023
@jtcohen6 jtcohen6 transferred this issue from dbt-labs/dbt-core Feb 15, 2024
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Feb 15, 2024

I believe that fixing this bug would enable a workaround for defining tags / masking policies as part of Snowflake model contracts:

models:
  - name: my_model
    config:
      contract: {enforced: true}
      materialized: table
    columns:
      - name: id
        data_type: int
        constraints:
          - type: custom
            expression: "tag (jerco_test_tag = 'some_value')"
            # or
            expression: "masking policy {{ target.database }}.{{ target.schema }}.jerco_masque"

By applying the tags/policies as part of the CTA, we avoid the risk of momentary delay between table creation and tag/policy application. That risk exists in an approach leveraging post-hooks, such as in the dbt_snow_mask package.

A better future answer would be natively supporting masking policies in CTAs, and helping users manage UDFs/policies as a first-class dbt node type / DWH object, but it feels like a viable way to unblock folks in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Medium Severity bug with minor impact that does not have resolution timeframe requirement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant