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

Field references inside and outside of the subquery should be distinguished #27735

Open
Tracked by #33413
metamben opened this issue Jan 17, 2023 · 4 comments
Open
Tracked by #33413
Labels
.Backend Priority:P2 Average run of the mill bug Querying/MBQL Querying/Nested Queries Questions based on other saved questions .Team/QueryProcessor :hammer_and_wrench: Type:Bug Product defects .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2

Comments

@metamben
Copy link
Contributor

Describe the bug
References to the results of nested queries is incorrect when the result is produced by applying a non-idempotent function.

For example, the following (nonsensical) query
image

is sent as the following MBQL query to the backend:

{:database 353,
  :query
  {:source-query
   {:source-table 866, :aggregation [["count"]], :breakout [["field" 4906 {:temporal-unit "month-of-year"}]]},
   :aggregation [["count"]],
   :breakout [["field" "LAST_LOGIN" {:base-type "type/DateTime", :temporal-unit "month-of-year"}]]},
  :type "query",
  :parameters [],
  :middleware {:js-int-to-string? true, :add-default-userland-constraints? true}}

The backend then tries to extract the month from the LAST_LOGIN result of the subquery which fails.

A more complicated example having the same problem can be found in #17769.

Potential solution

References inside the query should be distinguished from the references to the results of the subquery. For example,

;; for referring to the column at the same level of the query
field_ref => [:field 1 {:temporal-unit :month-of-year}]

;; for referring to the column if you nest the query
field_ref_source_query => [:field "my_field" {:base-type :type/Integer}]

Dependencies

Implementing the fix should be much easier after the completion of #proj-metabase-lib because then the changes will mostly be confined to the Clojure world and few additional frontend changes will be necessary.

@camsaul
Copy link
Member

camsaul commented Feb 28, 2023

I think we can address this as part of #28689

@camsaul
Copy link
Member

camsaul commented Aug 25, 2023

This is basically the same underlying issue as #29763

@ranquild ranquild self-assigned this Nov 8, 2023
@ranquild
Copy link
Contributor

ranquild commented Nov 8, 2023

The issue still exists. Steps with the sample DB:
Screenshot 2023-11-08 at 12 53 03
Screenshot 2023-11-08 at 12 53 07

@kamilmielnik
Copy link
Contributor

Reproducible in master at 78cb288.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Backend Priority:P2 Average run of the mill bug Querying/MBQL Querying/Nested Queries Questions based on other saved questions .Team/QueryProcessor :hammer_and_wrench: Type:Bug Product defects .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2
Projects
None yet
Development

No branches or pull requests

6 participants