-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(query): enable min/max pushdown #20994
Conversation
2141b37
to
43f72dc
Compare
86d9780
to
6d5235f
Compare
fe9745e
to
89cf604
Compare
89cf604
to
65c3851
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had some questions. I remember that we had a bug in Cloud with group
and selectors, so I want to be sure that we have coverage for that particular test case.
"PushDownRangeRule": 1, | ||
"PushDownFilterRule": 1, | ||
]) | ||
group_min_test.group_min_bare() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking at the test that this test extends, and I see that it's filtering by _value
:
https://github.com/influxdata/flux/blob/1f16d09633be04ace0b806bbaa90db59ee4ff15a/stdlib/planner/group_min_test.flux#L25
The test for max
is similar. This doesn't seem like it would actually validate that these functions work correctly. Am I missing something?
t.appendBounds(colReader) | ||
return true | ||
} | ||
|
||
type {{.name}}AggregateMethod func([]int64, []{{.Type}}) (int64, {{.Type}}) | ||
type {{.Name}}AggregateAccumulator interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wolffcm This file has a lot of the bug fix work that you were looking for.
a5383df
to
8a4b490
Compare
72e7186
to
fb21461
Compare
94919fb
to
d76cd24
Compare
d76cd24
to
fb29389
Compare
Yay! At last :) |
* feat(query): enable min/max pushdown * fix(query): fix the group last pushdown to use descending cursors * test(storage): add read group test with no agg Co-authored-by: Jonathan A. Sternberg <jonathan@influxdata.com>
Closes #