add missing parameter to term agg #2103
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #2103 +/- ##
==========================================
- Coverage 94.39% 94.31% -0.08%
==========================================
Files 321 321
Lines 60612 61262 +650
==========================================
+ Hits 57213 57782 +569
- Misses 3399 3480 +81
|
| let new_col = Column { | ||
| index: columnar::ColumnIndex::Full, | ||
| // Replace u64::MAX later with actual term | ||
| values: col.0.first_or_default_col(u64::MAX), |
There was a problem hiding this comment.
is u64::MAX mapping to NaN? Why is this working?
src/aggregation/bucket/term_agg.rs
Outdated
| bucket_agg_accessor | ||
| .column_block_accessor | ||
| .fetch_block(docs, &bucket_agg_accessor.accessor); | ||
| if let Some(missing) = bucket_agg_accessor.missing_accessor1 { |
There was a problem hiding this comment.
I don''t understand what happens when we have two columns of type string and int
For instance, if we have 3 docs.
doc1 -> "hello"
doc2 -> 3
doc3 -> missing
And we want a term aggregation with missing = "N/A".
With logic below, don't we end up with
"hello" -> 1
3 -> 1
"N/A" -> 3 (instead of 1)
There was a problem hiding this comment.
I added a comment for this on the aggregation request
missing parameter on term aggregation
Limitations:
Mixed types columns are not supported.
Missing key of type string is not supported on numerical fields
It requires a special missing aggregation to address those limitations
#1913
#1789