pkg/query: Set different metadata on aggregated row to null #3805
Conversation
|
🤖 Meticulous spotted visual differences in 1 of 333 screens tested: view and approve differences detected. Last updated for commit 4c54b72. This comment will update as new commits are pushed. |
metalmatze
force-pushed
the
flamegraph-arrow-setnull
branch
from
c7e297d to
a04e9dc
Compare
|
FrostDB is updated in this PR and the replace in the go.mod is only to get apache/arrow#37695 in. Once that merges we can simply remove that replace too. |
pkg/query/flamegraph_arrow.go
Outdated
| if fb.builderFunctionStartLine.IsValid(cr) { | ||
| a := fb.builderFunctionStartLine.Value(cr) | ||
| b := r.LineFunctionStartLine.Value(lineIndex) | ||
| if r.LineFunctionStartLine.IsNull(lineIndex) || a != b { |
There was a problem hiding this comment.
This is going to be a very extreme hot-path, so we're going to want to first check for null in every one of these branches, as reading the validity bitmap is much faster than reading the values, which we would be doing unnecessarily as is.
There was a problem hiding this comment.
Sure, I can change it proactively as I agree.
I was waiting to see what profiling was going to show.
Whenever we merge/aggregate functions we want to make sure that all meta data columns match with their content.
For example, if the underlying filename is different, we have to set that column's value to
null.This is to avoid showing misleading information.