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: fix joined table with aliases in metric #9842

Merged
merged 5 commits into from Apr 23, 2024
Merged

Conversation

rephus
Copy link
Collaborator

@rephus rephus commented Apr 22, 2024

Closes: #9726

Solution

Generate a mapping with all the joins tables and aliases so we can evaluate the alias in the dimension reference method

Screenshot from 2024-04-22 11-24-28

Description:

This PR is only fixing the specified issue with nested joins , alias and sql metric. There are other scenarios that are not covered on this PR , like dimensions, user attributes, etc. THe approach should be similar to this fix though.

Screenshot from 2024-04-22 12-21-47

Before
Screenshot from 2024-04-22 09-32-33

Screenshot from 2024-04-22 11-49-06

After:

Screenshot from 2024-04-22 12-21-03 (copy)

Screenshot from 2024-04-22 12-21-33

Reviewer actions

  • I have manually tested the changes in the preview environment
  • I have reviewed the code
  • I understand that "request changes" will block this PR from merging

@owlas owlas requested a deployment to fix-alias-joined-table - jaffle_db_pg_13 PR #9842 April 22, 2024 10:25 — with Render Abandoned
@owlas owlas deployed to fix-alias-joined-table - headless-browser PR #9842 April 22, 2024 10:25 — with Render Active
@rephus rephus requested a review from almeidabbm April 22, 2024 10:25
Copy link

netlify bot commented Apr 22, 2024

Deploy Preview for peaceful-bassi-cbf284 canceled.

Name Link
🔨 Latest commit 3945669
🔍 Latest deploy log https://app.netlify.com/sites/peaceful-bassi-cbf284/deploys/66275e3c6791a2000855f866

@owlas owlas temporarily deployed to fix-alias-joined-table - lightdash PR #9842 April 22, 2024 10:27 — with Render Destroyed
@owlas owlas deployed to fix-alias-joined-table - headless-browser PR #9842 April 22, 2024 10:29 — with Render Active
@almeidabbm
Copy link
Contributor

When trying to compile the same join alias you have on the screenshots using the CLI:

diff --git a/examples/full-jaffle-shop-demo/dbt/models/schema.yml b/examples/full-jaffle-shop-demo/dbt/models/schema.yml
index 624508de6..6118fd529 100755
--- a/examples/full-jaffle-shop-demo/dbt/models/schema.yml
+++ b/examples/full-jaffle-shop-demo/dbt/models/schema.yml
@@ -93,6 +93,7 @@ models:
     meta:
       joins:
         - join: customers
+          alias: foo
           sql_on: ${customers.customer_id} = ${orders.customer_id}
     columns:
       - name: order_id
@@ -102,6 +103,9 @@ models:
         description: This is a unique identifier for an order
         meta:
           metrics:
+            first_name_count:
+              type: count_distinct
+              field: ${foo.first_name} IS NOT NULL
             unique_order_count:
               type: count_distinct
             completed_or_shipped_order_count:

I am getting this error:

image

@rephus
Copy link
Collaborator Author

rephus commented Apr 23, 2024

When trying to compile the same join alias you have on the screenshots using the CLI:

Fixed, there might be more like this like I mentioned in the description

This PR is only fixing the specified issue with nested joins , alias and sql metric. There are other scenarios that are not covered on this PR , like dimensions, user attributes, etc. THe approach should be similar to this fix though.

@owlas owlas temporarily deployed to fix-alias-joined-table - lightdash PR #9842 April 23, 2024 07:07 — with Render Destroyed
@owlas owlas temporarily deployed to fix-alias-joined-table - headless-browser PR #9842 April 23, 2024 07:07 — with Render Destroyed
Copy link
Contributor

@almeidabbm almeidabbm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUPER!! LGTM 🚀

image

@rephus rephus merged commit 25aef88 into main Apr 23, 2024
46 of 49 checks passed
@rephus rephus deleted the fix-alias-joined-table branch April 23, 2024 09:03
lightdash-bot pushed a commit that referenced this pull request Apr 23, 2024
## [0.1075.1](0.1075.0...0.1075.1) (2024-04-23)

### Bug Fixes

* fix joined table with aliases in metric ([#9842](#9842)) ([25aef88](25aef88))
@lightdash-bot
Copy link
Collaborator

🎉 This PR is included in version 0.1075.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Table join aliases not resolved correctly during project compilation
4 participants