Skip to content
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

Merged
merged 8 commits into from
Nov 2, 2023
Merged

Conversation

damian3031
Copy link
Member

resolves #361

  • Support limiting get_catalog by object name
  • Added date_spine macro
  • added new tests from dbt-tests-adapter: dbt show, storing test failures as views, simple seed tests

dbt/include/trino/macros/catalog.sql Show resolved Hide resolved
"""


class TestDocsGenerateOverride:
Copy link
Member

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 ?

Copy link
Member Author

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)

Comment on lines +103 to +116
{% 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 %}
Copy link
Member

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

Copy link
Member Author

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).

Copy link
Member

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?

Copy link
Member Author

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

@damian3031 damian3031 changed the title Support limiting get_catalog by object name upgrade to support dbt-core v1.7.0 Oct 25, 2023
@damian3031 damian3031 merged commit 021a9ef into master Nov 2, 2023
11 checks passed
@damian3031 damian3031 deleted the add-dbt-core-1.7.0-features branch November 2, 2023 10:17
) %}
{% endif %}

{%- if not loop.last %} or {% endif -%}
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

upgrade to support dbt-core v1.7.0
3 participants