-
Notifications
You must be signed in to change notification settings - Fork 114
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
Support Delta constraints #71
Conversation
cc @jtcohen6 |
"+persist_docs": { | ||
"relation": True, | ||
"columns": True, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just wondering if we need to enable them? I'm feeling it's ok to enable automatically if there is such configs in schema.yml
.
I also feel a bit weird that the config named persist_docs
enables the constraints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just wondering if we need to enable them? I'm feeling it's ok to enable automatically if there is such configs in
schema.yml
.
Fair, I think there's good arguments to be made in either direction. From my perspective, meta
is usually for user's own metadata / annotation purposes, with no functional effect in dbt, until/unless they opt into that via an extension macro/package or custom config.
I also feel a bit weird that the config named
persist_docs
enables the constraints.
Agreed. Even if the mechanism to add these constraints is very very similar to persist_docs
, I'd rather see this enabled via another config. Users (and therefore adapter plugins) can define whichever configs they like. All that would require is another macro call within the materialization, right alongside the calls to persist_docs
:
{% do persist_docs(target_relation, model) %} | |
dbt-databricks/dbt/include/databricks/macros/materializations/incremental/incremental.sql
Lines 43 to 45 in 5d44dc6
{% do persist_docs(target_relation, model) %} | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense! I will use a different config for constraints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might also need it in snapshot.sql
?
dbt-databricks/dbt/include/databricks/macros/materializations/snapshot.sql
Lines 96 to 98 in 6dd2a8f
{% do persist_docs(target_relation, model) %} | |
13ca3c6
to
6dd2a8f
Compare
9b2a46e
to
d6ddbfb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Note, Delta constraints are only available in Databricks Runtime 7.4 and above. So DBR 7.3 LTS won't support this feature.
If it doesn't support this feature with DBR 7.3 LTS, we should mention it in README to say:
- The
dbt-databricks
adapter has been tested againstDatabricks SQL
andDatabricks runtime releases 9.1 LTS
and later.
or
- Delta constraints feature only works with
Databricks runtime releases 9.1 LTS
and later.
@superdupershant Which would you recommend, or other option?
Thanks! merging. |
### Description This PR adds support for [Delta Constraints](https://docs.databricks.com/delta/delta-constraints.html) with dbt table models and incremental models. It supports two types of constraints: model level [Check constraint](https://docs.databricks.com/delta/delta-constraints.html#check-constraint) and column level [not_null](https://docs.databricks.com/delta/delta-constraints.html#not-null-constraint) constraint. Users can specify model-level and column-level constraints under the `meta` field of the model config. For example: ```yaml # schema.yml models: - name: my_model meta: constraints: - name: id_greater_than_zero condition: id > 0 columns: - name: id - name: name meta: constraint: not_null ``` Constraints specified will be created if the `persist_constraints` config is enabled (default: false): ```sql -- my_model.sql {{ config( materailized='table', persist_constraints=True ) }} ... ``` Note, Delta constraints are only available in Databricks Runtime 7.4 and above. So DBR 7.3 LTS won't support this feature.
### Description We introduced Delta constraints at #71 and now that snapshot query could fail when it's violating the constraints. In that case, the temporary view keeps existing because snapshot uses a permanent view and drop it later. We should always drop them.
### Description We introduced Delta constraints at databricks#71 and now that snapshot query could fail when it's violating the constraints. In that case, the temporary view keeps existing because snapshot uses a permanent view and drop it later. We should always drop them.
Description
This PR adds support for Delta Constraints with dbt table models and incremental models. It supports two types of constraints: model level Check constraint and column level not_null constraint.
Users can specify model-level and column-level constraints under the
meta
field of the model config. For example:Constraints specified will be created if the
persist_constraints
config is enabled (default: false):Note, Delta constraints are only available in Databricks Runtime 7.4 and above. So DBR 7.3 LTS won't support this feature.
Checklist
CHANGELOG.md
and added information about my change to the "dbt-databricks next" section.