-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Return the correct auxiliary values for top/bottom #9858
Return the correct auxiliary values for top/bottom #9858
Conversation
During a run of megacheck the following issues were discovered:
|
query/functions.go
Outdated
heap.Fix(r.h, 0) | ||
fmt.Println(r.h.points) |
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.
Debug code?
query/functions.go
Outdated
var clone FloatPoint | ||
p.CopyTo(&clone) | ||
heap.Push(r.h, clone) | ||
fmt.Println(r.h.points) |
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.
Debug code?
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.
Removed.
30c0971
to
50614ec
Compare
During a run of megacheck the following issues were discovered:
|
CI is failing:
|
When `top()` or `bottom()` were used and selected auxiliary values, they would return the wrong values that would be equal to the last point selected. This is because the aggregators saved the memory address of the auxiliary fields instead of copying them over. Since the same auxiliary fields memory location is used for every value returned by the storage engine, this resulted in the values being incorrect because they were overwritten with incorrect values. This fixes that so the auxiliary fields are copied out when they are saved rather than only the memory location.
50614ec
to
8a2bc63
Compare
Maybe that explains why megacheck has been failing. Fixed. |
During a run of megacheck the following issues were discovered:
|
When
top()
orbottom()
were used and selected auxiliary values, theywould return the wrong values that would be equal to the last point
selected. This is because the aggregators saved the memory address of
the auxiliary fields instead of copying them over. Since the same
auxiliary fields memory location is used for every value returned by the
storage engine, this resulted in the values being incorrect because they
were overwritten with incorrect values.
This fixes that so the auxiliary fields are copied out when they are
saved rather than only the memory location.
Fixes #9197.