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

[MLv2] Add aggregation-column and breakout-column #37873

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

bshepherdson
Copy link
Contributor

These are helpers for going from an aggregation or breakout
clause to the relevant :metadata/column they are based on.

Eg. the column being SUM'd by an aggregation or bucketed by a breakout.

(Nil for top-level aggregations like :count.)

Fixes #37120.

Copy link
Contributor Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@metabase-bot metabase-bot bot added the .Team/QueryProcessor :hammer_and_wrench: label Jan 18, 2024
@bshepherdson bshepherdson added the no-backport Do not backport this PR to any branch label Jan 18, 2024
@bshepherdson bshepherdson added this to the 0.49 milestone Jan 18, 2024
These are helpers for going from an aggregation or breakout
clause to the relevant `:metadata/column`. (Nil for top-level
aggregations like `:count`.)
@bshepherdson bshepherdson force-pushed the mblib-aggregation-breakout-column branch from e4a9905 to 145ad1a Compare January 18, 2024 20:44
Copy link
Contributor

@metamben metamben left a comment

Choose a reason for hiding this comment

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

LGTM. Does this have to work only when the column is one level deep (i.e., not when we sum the double of column)?

@bshepherdson
Copy link
Contributor Author

Does this have to work only when the column is one level deep (i.e., not when we sum the double of column)?

I would argue that "column" is not "field" - if one is doubling a column A that's an expression E. SUM(E)'s column of interest is E, not A, which is what gets returned here.

@bshepherdson bshepherdson merged commit 7837c71 into master Jan 19, 2024
106 checks passed
@bshepherdson bshepherdson deleted the mblib-aggregation-breakout-column branch January 19, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport Do not backport this PR to any branch .Team/QueryProcessor :hammer_and_wrench:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MLv2] [BE] Add breakoutColumn and aggregationColumn APIs to simply columns retrieval from these clauses
2 participants