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.

Sign up for GitHub

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

Jump to bottom

🤖 backported "source-field populated in aggregates with foreign refs" #38667

Merged
merged 1 commit into from
Feb 12, 2024
Merged

🤖 backported "source-field populated in aggregates with foreign refs" #38667

merged 1 commit into from
Feb 12, 2024

Conversation

metabase-bot[bot]
Copy link
Contributor

@metabase-bot metabase-bot bot commented Feb 12, 2024

@markbastian
source-field populated in aggregates with foreign refs (#38624)
06d3215 
* source-field populated in aggregates with foreign refs

When aggregates are populated that reference other tables, either a `:source-field` must be provided in the field metadata or a `:join-alias` needs to be provided. The former is preferred.

Prior to this PR, the first situation in the below simplified case could happen:

```clojure
;;Simplified case
;; busted
(qp/process-query
  {:database 1,
   ;; 59 is "PRODUCTS" -> "PRICE" which is not in the "REVIEWS" table
   :query    {:aggregation  [["sum" ["field" 59 {:base-type "type/Float"}]]]
              ;; "REVIEWS"
              :source-table 8},
   :type     "query"})

; Works
(qp/process-query
  {:database 1,
   ;; The `:source-field` metadata fixes the above query
   :query    {:aggregation  [["sum" ["field" 59 {:base-type    "type/Float"
                                                 ;; "REVIEWS" -> "PRODUCT_ID"
                                                 :source-field 71}]]]
              :source-table 8},
   :type     "query"})
```

This PR updates `metabase.automagic-dashboards.interestin/ground-metric` to use the `->reference` function for fields when building the `decoder` for transforming field names to references, which properly adds sources for external fields.

Fixes #38618

* Updating `grounded-metrics-test` to reflect updated field referencing.
@metabase-bot metabase-bot bot requested a review from camsaul as a code owner February 12, 2024 14:42
@metabase-bot metabase-bot bot added the was-backported apply this to PRs that are themselves backports label Feb 12, 2024
@metabase-bot metabase-bot bot enabled auto-merge (squash) February 12, 2024 14:42
@replay-io replay.io
Copy link

replay-io bot commented Feb 12, 2024 •
edited

Status Complete ↗︎
Commit 06d3215
Results
⚠️ 3 Flaky
✅ 2302 Passed

@metabase-bot metabase-bot bot merged commit 1730e3d into release-x.49.x Feb 12, 2024
106 checks passed
@metabase-bot metabase-bot bot deleted the backport-a9b792c92d0960942973c7bc484691484768d333 branch February 12, 2024 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions approved these changes

@camsaul camsaul Awaiting requested review from camsaul camsaul is a code owner

Assignees

@markbastian markbastian

Labels
was-backported apply this to PRs that are themselves backports
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@markbastian