-
Notifications
You must be signed in to change notification settings - Fork 138
Add column mask support for tables and incremental #1033
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
Conversation
20396e6
to
17a5d5c
Compare
17a5d5c
to
ef90eba
Compare
c6ab091
to
4c15afc
Compare
@@ -12,6 +12,7 @@ | |||
|
|||
{{ apply_alter_constraints(target_relation) }} | |||
{{ apply_tags(target_relation, tags) }} | |||
{{ apply_column_masks_from_model_columns(target_relation) }} |
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.
Since this is not performed as part of the same transaction as the table creation, I believe there could be a period of time where the masks are not applied. Based on https://docs.databricks.com/aws/en/sql/language-manual/sql-ref-syntax-ddl-column-mask, it is technically possible to apply masks at table creation
If this is preferred, I could make that change
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.
Yes, on create we should apply at create time. That's the reason we are restricting the feature to Mat v2, because in v1, the presence of 'select' prevents us from doing column stuff.
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.
Addressed this in d62217f
Let's discuss this in our 1:1 so I can understand it better. |
a4010d7
to
443a2ef
Compare
Resolves #670
Description
column_mask
New config example
Executed SQL:
Note that
id
is a reference to the columnid
of the same table.'hello'
is a string literalLimitations
When executing the following steps for incremental materializations, I ran into issues
mask_function(value STRING)
mask_function(value STRING, another_value STRING)
using_columns
to pass in the extra argumentThe above will fail because applying the changeset (i.e. executing the
ALTER TABLE
statement to update the mask) is done later than the firstSELECT
statement that runs against the table. This results in an error likeLooking at the macros, handling this edge case seems like it will need significant refactors. Not sure if it is worth the effort, so I have instead added a runtime error that is thrown when detecting that an existing function's signature has changed
UPDATE: we will not try to do anything fancy to handle this edge case. We consider this a user error as changing the function signature puts the table in a broken state. We will document this clearly in the doc update
Checklist
CHANGELOG.md
and added information about my change to the "dbt-databricks next" section.