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

Fix duplicate input measures error #968

Merged
merged 3 commits into from
Jan 11, 2024
Merged

Conversation

WilliamDee
Copy link
Contributor

@WilliamDee WilliamDee commented Jan 10, 2024

Resolves #969

Description

Currently, when running through the branch combiner, in an instance where you have 2 derived metrics that contains the same input measure, it would get combined and it doesn't get deduped. Example,

metric:
  name: "derived1"
  description: Test child derived metric 1
  type: derived
  type_params:
    expr: "booking_value + bookings"
    metrics:
      - name: booking_value
      - name: bookings
---
metric:
  name: "derived2"
  description: Test child derived metric 2
  type: derived
  type_params:
    expr: "bookings + instant_bookings"
    metrics:
      - name: instant_bookings
      - name: bookings
---
metric:
  name: "derived_parent"
  description: Test parent derived metric
  type: derived
  type_params:
    expr: "derived1 + derived2"
    metrics:
      - name: derived1
      - name: derived2

This leads to hitting an error during the dataflow rendering where we matched 2 duplicate input measures when expecting only 1 (here). Additionally, the parser in dbt-core utilizes it's own rewritten set of transformation rules and it currently isn't doing any deduping, this is a bug and should be fixed within dbt-core, tho it isn't affecting this issue.

NOTE: this error only occurs from manifests parsed by dbt-core, which is why the error never popped up in MF as MF uses the transformation rules in DSI.

  1. Dedupe the combined input measure specs
  2. Dedupe metric input measures in transformation rule (to remove after change is in core)

@cla-bot cla-bot bot added the cla:yes label Jan 10, 2024
Copy link

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.

@WilliamDee WilliamDee requested review from plypaul and tlento January 10, 2024 20:46
@WilliamDee WilliamDee force-pushed the will/fix-dupe-measure-input branch from 5d8517e to 6bbbcab Compare January 11, 2024 20:24
@WilliamDee WilliamDee merged commit 684b97b into main Jan 11, 2024
9 checks passed
@WilliamDee WilliamDee deleted the will/fix-dupe-measure-input branch January 11, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SL-1520] [Bug] Duplicate input measure matched in plan rendering
2 participants