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

[CT-3073] [Bug] semantic model doesn't work in the "plus" node selection #8546

Closed
2 tasks done
Tracked by #8125
popcornylu opened this issue Sep 5, 2023 · 1 comment · Fixed by #8589
Closed
2 tasks done
Tracked by #8125

[CT-3073] [Bug] semantic model doesn't work in the "plus" node selection #8546

popcornylu opened this issue Sep 5, 2023 · 1 comment · Fixed by #8589
Assignees
Labels
backport 1.6.latest bug Something isn't working Impact: Exp semantic Issues related to the semantic layer
Milestone

Comments

@popcornylu
Copy link

popcornylu commented Sep 5, 2023

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

dbt list -s 'stg_products+' 

does not include any metrics

Expected Behavior

dbt list -s 'stg_products+' 

should include all metrics

Steps To Reproduce

  1. Clone the metric example project https://github.com/dbt-labs/jaffle-sl-template
  2. The lineage graph is as follow
    image
  3. Run dbt list -s 'stg_products+', and no metrics are not in the list (error)
  4. Run dbt list -s 'metric:jaffle_shop.revenue+' we can see four metrics (correct)

It seems the semantic models are not considered in the plus operator

Relevant log output

$ dbt list -s 'stg_products+'
06:06:02  Running with dbt=1.6.1
06:06:02  Registered adapter: duckdb=1.6.0
06:06:02  Found 10 models, 6 seeds, 18 tests, 6 sources, 0 exposures, 14 metrics, 563 macros, 0 groups, 5 semantic models
jaffle_shop.marts.customers
jaffle_shop.marts.order_items
jaffle_shop.marts.orders
jaffle_shop.staging.stg_products
jaffle_shop.marts.accepted_values_customers_customer_type__new__returning
jaffle_shop.marts.not_null_customers_customer_id
jaffle_shop.marts.not_null_orders_order_id
jaffle_shop.staging.not_null_stg_products_product_id
jaffle_shop.marts.relationships_orders_customer_id__customer_id__ref_stg_customers_
jaffle_shop.marts.unique_customers_customer_id
jaffle_shop.marts.unique_orders_order_id

Environment

- OS:MacOS 13.2.1
- Python:3.10.11
- dbt: 1.6.1

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

duckdb

@popcornylu popcornylu added bug Something isn't working triage labels Sep 5, 2023
@github-actions github-actions bot changed the title [Bug] semantic model doesn't work in the "plus" node selection [CT-3073] [Bug] semantic model doesn't work in the "plus" node selection Sep 5, 2023
jtcohen6 added a commit that referenced this issue Sep 5, 2023
@jtcohen6
Copy link
Contributor

jtcohen6 commented Sep 5, 2023

@popcornylu Thanks for opening! Agree that this is a bug, and not the intended behavior.

I believe the basic problem here is that we're calling add_node here, instead of link_node:

for semantic_model in manifest.semantic_models.values():
self.add_node(semantic_model.unique_id)

We only use add_node for sources. We use link_node for any nodes that can depend on other nodes, including metrics.

In addition to fixing the graph compilation, we also need to support semantic models within the output of dbt list. I started roughing in what a fix could look like: 025d3dc

Using the changes in that commit:

$ dbt list -s stg_products+
09:46:37  Running with dbt=1.7.0-b2
09:46:37  Registered adapter: postgres=1.7.0-b2
09:46:37  Found 10 models, 6 seeds, 18 tests, 6 sources, 0 exposures, 14 metrics, 595 macros, 0 groups, 5 semantic models
metric:jaffle_shop.cumulative_revenue
metric:jaffle_shop.customers_with_orders
metric:jaffle_shop.food_orders
metric:jaffle_shop.food_revenue
metric:jaffle_shop.food_revenue_pct
metric:jaffle_shop.large_order
metric:jaffle_shop.median_revenue
metric:jaffle_shop.new_customer
metric:jaffle_shop.order_cost
metric:jaffle_shop.order_gross_profit
metric:jaffle_shop.order_total
metric:jaffle_shop.orders
metric:jaffle_shop.revenue
metric:jaffle_shop.revenue_growth_mom
jaffle_shop.marts.customers
jaffle_shop.marts.order_items
jaffle_shop.marts.orders
jaffle_shop.staging.stg_products
semantic_model:jaffle_shop.customers
semantic_model:jaffle_shop.order_item
semantic_model:jaffle_shop.orders
semantic_model:jaffle_shop.stg_products
jaffle_shop.marts.accepted_values_customers_customer_type__new__returning
jaffle_shop.marts.not_null_customers_customer_id
jaffle_shop.marts.not_null_orders_order_id
jaffle_shop.staging.not_null_stg_products_product_id
jaffle_shop.marts.relationships_orders_customer_id__customer_id__ref_stg_customers_
jaffle_shop.marts.unique_customers_customer_id
jaffle_shop.marts.unique_orders_order_id
jaffle_shop.staging.unique_stg_products_product_id

Acceptance criteria

  1. Replace add_node with link_node for semantic models.
  2. Since semantic models can have the same name as models (e.g. jaffle_shop.staging.stg_products), they must be visually distinct in the default list output.
  3. Add support for a semantic_model:{name} selection syntax, similar to what we support for source:, exposure:, and metric:. I'd love to see us generalize this to just {resource_type}:{fully_qualified_name}, and then every resource output by dbt list would include its resource type. I don't see why we couldn't do that!

(1) is the narrowest fix for the bug, but if we did only (1), it would not be a desirable product experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.6.latest bug Something isn't working Impact: Exp semantic Issues related to the semantic layer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants