-
Notifications
You must be signed in to change notification settings - Fork 64
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 support for binaryOps for native histograms #526
add support for binaryOps for native histograms #526
Conversation
Signed-off-by: Saumya Shah <[email protected]>
Signed-off-by: Saumya Shah <[email protected]>
Signed-off-by: Saumya Shah <[email protected]>
What about the other many to many joins "and"and "unless" |
Yes, "unless" needs to be implemented as well. Though, the implementation of end exists (and with current "or" implementation), ig it needs to handle few cases !??
but in the below query:
o/p from thanos engine will be empty based on how we are handling here but as labels of lhs and rhs matches, the o/p should be the answer of such queries is redundant though 😅 no idea if this is something which should be changed or not though |
Signed-off-by: Saumya Shah <[email protected]>
Signed-off-by: Saumya Shah <[email protected]>
What happens if native histogram and float series collide in same hash bucket? Can you add a test for that? We should behave same as Prometheus here even if the query might look weird |
Signed-off-by: Saumya Shah <[email protected]>
Yup, made same changes where |
Signed-off-by: Saumya Shah <[email protected]>
Can you please add some testcases for mixed (histogram vs float) many-to-many matches just to be super sure that those work as intended? Code itself looks solid to me and from running through it in my head i didnt see anything wrong so i would think those test cases will jsut work - but we should have them regardless |
…ins in tests Signed-off-by: Saumya Shah <[email protected]>
Signed-off-by: Saumya Shah <[email protected]>
I've added few cases where lhs and rhs are can be (native histogram or a float series with different/no-labels), does it needs any changes? |
Found via #525
Prometheus supports the binary OR, AND, UNLESS operation when a native histogram is there in the query, this PR updates the related functions to take in account for native histograms.