fix handling of bytes for cardinality agg

This commit is contained in:
trinity.pointard
2026-07-03 14:56:33 +00:00
parent ba1a979824
commit be7f09a0b2
2 changed files with 46 additions and 4 deletions

View File

@@ -756,7 +756,7 @@ fn build_multi_terms_node(
let str_dict_column = reader.fast_fields().str(field_name)?;
// Collect all columns for this field (handles JSON multi-type fields).
let columns = get_term_agg_accessors(reader, field_name, &field_def.missing)?;
let columns = get_term_agg_accessors(reader, field_name, &field_def.missing, true)?;
// Precompute the missing KeyElem (or None -> drop combo).
let missing_key_elem = if let Some(missing) = &field_def.missing {
@@ -868,10 +868,15 @@ fn get_term_agg_accessors(
reader: &SegmentReader,
field_name: &str,
missing: &Option<Key>,
include_bytes: bool,
) -> crate::Result<Vec<(Column<u64>, ColumnType)>> {
// `terms` and `multi_terms` both explicitly reject `Bytes` columns downstream, which needs
// to actually see them as a real column (rather than the empty shim below) to do so.
let allowed_column_types = [
// `cardinality` has no such rejection: it would hash raw `Bytes` term ordinals as if they
// were comparable numeric values, but those ordinals are segment-local, so distinct byte
// values in different segments could collide and be undercounted. Keep `Bytes` out of its
// accessors entirely instead.
let mut allowed_column_types = vec![
ColumnType::I64,
ColumnType::U64,
ColumnType::F64,
@@ -879,8 +884,10 @@ fn get_term_agg_accessors(
ColumnType::DateTime,
ColumnType::Bool,
ColumnType::IpAddr,
ColumnType::Bytes,
];
if include_bytes {
allowed_column_types.push(ColumnType::Bytes);
}
// In case the column is empty we want the shim column to match the missing type
let fallback_type = missing
@@ -932,7 +939,8 @@ fn build_terms_or_cardinality_nodes(
let str_dict_column = reader.fast_fields().str(field_name)?;
let column_and_types = get_term_agg_accessors(reader, field_name, missing)?;
let include_bytes = matches!(req, TermsOrCardinalityRequest::Terms(_));
let column_and_types = get_term_agg_accessors(reader, field_name, missing, include_bytes)?;
// Special handling when missing + multi column or incompatible type on text/date.
let missing_and_more_than_one_col = column_and_types.len() > 1 && missing.is_some();

View File

@@ -1119,6 +1119,40 @@ mod tests {
Ok(())
}
#[test]
fn cardinality_aggregation_bytes_excluded_from_accessors() -> crate::Result<()> {
// `Bytes` columns are opened as raw per-segment dictionary ordinals (like `Str`), but
// unlike `Str`, cardinality has no dictionary-resolution path for them: it would hash
// the raw ordinal directly, which are segment dependant. Ignore bytes values instead of
// counting them wrong.
let mut schema_builder = Schema::builder();
let field = schema_builder.add_bytes_field("raw", FAST);
let index = Index::create_in_ram(schema_builder.build());
{
let mut writer = index.writer_for_tests()?;
writer.add_document(doc!(field => vec![1u8]))?;
writer.add_document(doc!(field => vec![2u8]))?;
writer.commit()?;
writer.add_document(doc!(field => vec![3u8]))?;
writer.add_document(doc!(field => vec![4u8]))?;
writer.commit()?;
}
let agg_req: Aggregations = serde_json::from_value(json!({
"cardinality": {
"cardinality": {
"field": "raw"
},
}
}))
.unwrap();
let res = exec_request(agg_req, &index)?;
assert_eq!(res["cardinality"]["value"], 0.0);
Ok(())
}
#[test]
fn cardinality_aggregation_json() -> crate::Result<()> {
let mut schema_builder = Schema::builder();