Bumping version to 1.5.0rc2, Add model contracts #286
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
damian3031
changed the title
damian3031
force-pushed
the
upgrade-dbt-core-to-1_5_0
branch
2 times, most recently
from
b9f8bff
to
28fc422
Compare
For some tests, we're receiving this error on galaxy: I made only these tests disabled on galaxy. I've duplicated them, decorated with |
|
Yes, it's known limitation. Maybe you could just adjust |
damian3031
force-pushed
the
upgrade-dbt-core-to-1_5_0
branch
from
28fc422
to
dc2afc9
Compare
@hovaesco adjusted to this comment |
Hmm wouldn't it be better to prepare PR, but submit it just after releasing dbt-trino 1.5.0 final? Maybe we shouldn't update dbt docs before releasing our stuff? To avoid situation where someone is asking about feature mentioned in docs, but not released yet |
damian3031
changed the title
dbt/adapters/trino/impl.py
Outdated
| @@ -21,6 +22,14 @@ class TrinoAdapter(SQLAdapter): | |||
| ConnectionManager = TrinoConnectionManager | |||
| AdapterSpecificConfigs = TrinoConfig | |||
|
|
|||
| CONSTRAINT_SUPPORT = { | |||
| ConstraintType.check: ConstraintSupport.NOT_SUPPORTED, | |||
| ConstraintType.not_null: ConstraintSupport.NOT_SUPPORTED, | |||
There was a problem hiding this comment.
If this works in iceberg, it would be nice to support this.
There was a problem hiding this comment.
I added new commit with support for not-null constraint. All tests needed to be marked with @pytest.mark.iceberg now, as catalog memory does not support non-null column
| create table | ||
| {{ relation }} | ||
| {{ get_table_columns_and_constraints() }} | ||
| {{ get_assert_columns_equivalent(sql) }} | ||
| {%- set sql = get_select_subquery(sql) %} |
There was a problem hiding this comment.
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 CREATE OR REPLACE.
There was a problem hiding this comment.
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.
As discussed offline, let's keep current implementation and I'll create issue which you mentioned.
This PR is about:
(High level explanation of these changes in this discussion under
Model Contractssection)Some remarks about model contracts implementation:
It is implemented by running 2 queries:
Create tableandInsert into(similar as in dbt-redshift - example query).It couldn't be implemeted as single CTAS query (as in dbt-snowflake - example query), as in Trino we can't
define column types in CTAS query.
dbt-core model contracts works on enforcing column names and types in table, and columns constraints:
not null,unique,primary key,foreign key,checkTrino doesn't support any of them, except that
not nullis supported in some connectors.It is a tricky one, as
not nullis supported in some connectors, but it isn't in others (memory, hive).It isn't documented in Trino docs (it isn't explicitly mentioned in connector page - hive connector docs, nor in CREATE TABLE docs)
So I'm not sure if we should support it now, as user would ask questions why it's not working with
'X' catalog, when we can't point them directly to connector documentation.
Please let me know what you think.