-
Notifications
You must be signed in to change notification settings - Fork 56
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
upgrade to support dbt-core v1.7.0 #368
Conversation
054daba
to
2645c8d
Compare
""" | ||
|
||
|
||
class TestDocsGenerateOverride: |
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.
is there a dbt test we could inherit from ?
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.
Unfortunately, not: dbt-labs/dbt-core#8307 (comment)
{% if relation.schema and relation.identifier %} | ||
( | ||
schema_name = '{{ relation.schema | lower }}' | ||
and table_name = '{{ relation.identifier | lower }}' | ||
) | ||
{% elif relation.schema %} | ||
( | ||
schema_name = '{{ relation.schema | lower }}' | ||
) | ||
{% else %} | ||
{% do exceptions.raise_compiler_error( | ||
'`get_catalog_relations` requires a list of relations, each with a schema' | ||
) %} | ||
{% endif %} |
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.
are you testing all cases here? same for trino__get_catalog_schemas_where_clause_sql
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.
The only test from dbt-tests-adapter
is TestDocsGenerateOverride. As mentioned in the comments, better tests are needed in dbt-tests-adapter
, and dbt has committed to prioritizing their creation (here).
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.
so are we waiting for these tests before merging this PR?
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 don't think so, as they probably won't be added in final release 1.7.0
, but rather in some patch release, hopefully in 1.7.1
2645c8d
to
eab77d5
Compare
bea795f
to
bfbd441
Compare
) %} | ||
{% endif %} | ||
|
||
{%- if not loop.last %} or {% endif -%} |
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.
note: the OR currently prevents Trino from optimizing these queries reasonably.
if this new code is a new to-be-common pattern in DBT, we may want to optimize information_schema.tables
and information_schema.columns
processing with this use-case in mind
resolves #361
get_catalog
by object namedbt show
, storing test failures as views, simple seed tests