-
Notifications
You must be signed in to change notification settings - Fork 53
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
Bumping version to 1.5.0rc2, Add model contracts #286
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
kind: Features | ||
body: Add model contracts | ||
time: 2023-04-19T11:52:08.343309+02:00 | ||
custom: | ||
Author: damian3031 | ||
Issue: "284" | ||
PR: "286" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
version = "1.4.2" | ||
version = "1.5.0rc2" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,11 +91,33 @@ | |
|
||
{% macro trino__create_table_as(temporary, relation, sql) -%} | ||
{%- set _properties = config.get('properties') -%} | ||
create table {{ relation }} | ||
|
||
{%- set contract_config = config.get('contract') -%} | ||
{%- if contract_config.enforced -%} | ||
|
||
create table | ||
{{ relation }} | ||
{{ get_table_columns_and_constraints() }} | ||
{{ get_assert_columns_equivalent(sql) }} | ||
{%- set sql = get_select_subquery(sql) %} | ||
Comment on lines
+98
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please create an issue to investigate type specification in CTAS in Trino. We should validate if this is supported by the SQL spec or not because this way won't allow to hot swap the table using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I would prefer this to be implemented with DESCRIBE OUTPUT instead of splitting into a CT and INSERT. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed offline, let's keep current implementation and I'll create issue which you mentioned. |
||
{{ properties(_properties) }} | ||
as ( | ||
{{ sql }} | ||
); | ||
; | ||
|
||
insert into {{ relation }} | ||
( | ||
{{ sql }} | ||
) | ||
; | ||
|
||
{%- else %} | ||
|
||
create table {{ relation }} | ||
{{ properties(_properties) }} | ||
as ( | ||
{{ sql }} | ||
); | ||
|
||
{%- endif %} | ||
{% endmacro %} | ||
|
||
|
||
|
@@ -108,6 +130,10 @@ | |
{% endif %} | ||
create or replace view | ||
{{ relation }} | ||
{%- set contract_config = config.get('contract') -%} | ||
{%- if contract_config.enforced -%} | ||
{{ get_assert_columns_equivalent(sql) }} | ||
{%- endif %} | ||
security {{ view_security }} | ||
as | ||
{{ sql }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
# model breaking constraints | ||
trino_model_char_value_to_int_column = """ | ||
{{ | ||
config( | ||
materialized = "table" | ||
) | ||
}} | ||
|
||
select | ||
-- char value for 'id', which is integer type | ||
'char_value' as id, | ||
-- change the color as well (to test rollback) | ||
'red' as color, | ||
'2019-01-01' as date_day | ||
""" | ||
|
||
trino_model_schema_yml = """ | ||
version: 2 | ||
models: | ||
- name: my_model | ||
config: | ||
contract: | ||
enforced: true | ||
columns: | ||
- name: id | ||
quote: true | ||
data_type: integer | ||
description: hello | ||
constraints: | ||
- type: check | ||
expression: (id > 0) | ||
tests: | ||
- unique | ||
- name: color | ||
data_type: varchar | ||
- name: date_day | ||
data_type: varchar | ||
- name: my_model_error | ||
config: | ||
contract: | ||
enforced: true | ||
columns: | ||
- name: id | ||
data_type: integer | ||
description: hello | ||
constraints: | ||
- type: check | ||
expression: (id > 0) | ||
tests: | ||
- unique | ||
- name: color | ||
data_type: varchar | ||
- name: date_day | ||
data_type: varchar | ||
- name: my_model_wrong_order | ||
config: | ||
contract: | ||
enforced: true | ||
columns: | ||
- name: id | ||
data_type: integer | ||
description: hello | ||
constraints: | ||
- type: check | ||
expression: (id > 0) | ||
tests: | ||
- unique | ||
- name: color | ||
data_type: varchar | ||
- name: date_day | ||
data_type: varchar | ||
- name: my_model_wrong_name | ||
config: | ||
contract: | ||
enforced: true | ||
columns: | ||
- name: id | ||
data_type: integer | ||
description: hello | ||
constraints: | ||
- type: check | ||
expression: (id > 0) | ||
tests: | ||
- unique | ||
- name: color | ||
data_type: varchar | ||
- name: date_day | ||
data_type: varchar | ||
""" | ||
|
||
trino_constrained_model_schema_yml = """ | ||
version: 2 | ||
models: | ||
- name: my_model | ||
config: | ||
contract: | ||
enforced: true | ||
constraints: | ||
- type: check | ||
expression: (id > 0) | ||
- type: primary_key | ||
columns: [ id ] | ||
- type: unique | ||
columns: [ color, date_day ] | ||
name: strange_uniqueness_requirement | ||
columns: | ||
- name: id | ||
quote: true | ||
data_type: integer | ||
description: hello | ||
constraints: | ||
- type: not_null | ||
tests: | ||
- unique | ||
- name: color | ||
data_type: varchar | ||
- name: date_day | ||
data_type: varchar | ||
""" |
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.
If this works in iceberg, it would be nice to support this.
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 added new commit with support for
not-null
constraint. All tests needed to be marked with@pytest.mark.iceberg
now, as catalogmemory
does not support non-null column