-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Add dimension selector support for groupby/having filters #2043
Conversation
@@ -0,0 +1,113 @@ | |||
package io.druid.query.groupby.having; |
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.
header
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.
done
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.
can we use this new header? See: #2050
can you also add a test to GroupByQueryRunnerTest, just to make sure the entire groupBy query works as expected ? |
Adding a testcase to GroupByQueryRunnerTest is nice to have but I feel not needed for this change. |
|
||
public byte[] getCacheKey() | ||
{ | ||
byte[] dimBytes = this.dimension.getBytes(Charsets.UTF_8); |
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.
StringUtils.toUtf8
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.
done
After reading #1984 I am 👍 on this. Please squash commits. |
2803566
to
d7ce120
Compare
👍 |
Add dimension selector support for groupby/having filters
No description provided.