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

Can't aggregate if the source question is an aggregated question #24839

Closed
qnkhuat opened this issue Aug 17, 2022 · 10 comments · Fixed by #33948
Closed

Can't aggregate if the source question is an aggregated question #24839

qnkhuat opened this issue Aug 17, 2022 · 10 comments · Fixed by #33948
Labels
.Backend Priority:P2 Average run of the mill bug Querying/Nested Queries Questions based on other saved questions .Reproduced Issues reproduced in test (usually Cypress) .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
Milestone

Comments

@qnkhuat
Copy link
Contributor

qnkhuat commented Aug 17, 2022

Describe the bug
There are no fields for aggregation in questions if the source question is aggregated

To Reproduce

  1. Question -> Sample -> Orders - Summary "Sum of Quantity" And "Average of total", grouped by "Created At:Month"
    Screen Shot 2022-08-17 at 20 44 01
  2. Save Question - Q1
  3. Create a new question with the source "Q1"
  4. Click Summarize -> Sum of : Notice there are no fields in the dropdown
    Screen Shot 2022-08-17 at 20 49 42

Expected behavior
When clicking Summarize -> Sum of, there should be 2 fields: "Sum of Quantity" And "Average of Total"

Screenshots
If applicable, add screenshots to help explain your problem.

Information about your Metabase Installation:
Current master(c41ecda)

Severity
P2?

Additional info
A workaround is to turn the question into a model, but we also have a bug(#23248) with the model.

@qnkhuat qnkhuat added Type:Bug Product defects .Needs Triage labels Aug 17, 2022
@flamber
Copy link
Contributor

flamber commented Aug 17, 2022

It only happens if you don't run the query (Visualize) before saving in step 2. I wrote more about it #16671 (comment).
It will cause other types of problems, dashboard filter selections etc: https://github.com/metabase/metabase/issues?q=is%3Aissue+is%3Aopen+%22result_metadata%22

@flamber flamber added Priority:P2 Average run of the mill bug Querying/Nested Queries Questions based on other saved questions .Backend and removed .Needs Triage labels Aug 17, 2022
@qnkhuat
Copy link
Contributor Author

qnkhuat commented Aug 17, 2022

Could you try again?
I tried to visualize then save, but it's still happening.

@flamber
Copy link
Contributor

flamber commented Aug 17, 2022

@qnkhuat If you refresh the question after you have saved, then it works. Not quite sure what is going on here.

@qnkhuat
Copy link
Contributor Author

qnkhuat commented Aug 22, 2022

Did some debugging and I think the problem is if BE returns result_metadata with a source="aggregation", then FE will fail to show the list.

The reason running the question before creating a new nested question works is if you run a question, BE updates the result_metadata that will remove the source property of metadata.

I think we need to check the FE to see why the presence of source creates this bug.

@daltojohnso
Copy link
Contributor

daltojohnso commented Sep 8, 2022

This seems not to be backend and as far as I can tell -- this is intentional on the part of the FE -- well, it seems intentional to me, but the code is four years old.

We call this StructuredQuery method to generate the list of fields for this popover:

aggregationFieldOptions(agg: string | AggregationOperator): DimensionOptions {

That method calls this StructuredQuery method to generate a list of fields:

fieldOptions(fieldFilter: FieldFilter = field => true): DimensionOptions {

There's a filter in that method to check to make sure that the field is not an aggregation:

export const isDimension = col =>

I think the reason this ever worked is because the FE terribly scrapes together various bits of field metadata and fails to include certain properties sometimes. I encountered this when testing #25109 and was surprised at the behavior and it took me a while to figure things out.

Is this intentional or should we open things up?

@nemanjaglumac

This comment was marked as duplicate.

@qnkhuat

This comment was marked as duplicate.

@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Dec 23, 2022
nemanjaglumac added a commit that referenced this issue Dec 24, 2022
* Repro #24839: Can't aggregate if source is the aggregated question (#27396)

* Repro #26861: "Exclude" field filter breaks native query (#27400)

* Repro #25994: Between specific dates filter fails after aggregation (#27404)
@ranquild ranquild self-assigned this Jan 23, 2023
@ranquild
Copy link
Contributor

I believe it should be fixed on the backend. There should be no such case that there is a different source before and after the query for first time.

@ranquild ranquild removed their assignment Jan 23, 2023
@darksciencebase darksciencebase added the .Team/QueryProcessor :hammer_and_wrench: label Mar 22, 2023
@darksciencebase darksciencebase added the .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2 label Jun 12, 2023
This was referenced Feb 5, 2024
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/Nested Queries Questions based on other saved questions .Reproduced Issues reproduced in test (usually Cypress) .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

Successfully merging a pull request may close this issue.

8 participants