Skip to content

Commit 657f0cd

Browse files
authored
add missing Bytes validation to term_agg (#2077)
returns empty for now instead of failing like before
1 parent 3a82ef2 commit 657f0cd

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

src/aggregation/agg_req_with_accessor.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,14 @@ impl AggregationWithAccessor {
7474
ColumnType::I64,
7575
ColumnType::U64,
7676
ColumnType::F64,
77-
ColumnType::Bytes,
7877
ColumnType::Str,
78+
// ColumnType::Bytes Unsupported
7979
// ColumnType::Bool Unsupported
8080
// ColumnType::IpAddr Unsupported
8181
// ColumnType::DateTime Unsupported
8282
];
8383
let mut columns =
84-
get_all_ff_reader(reader, field_name, Some(&allowed_column_types))?;
84+
get_all_ff_reader_or_empty(reader, field_name, Some(&allowed_column_types))?;
8585
let first = columns.pop().unwrap();
8686
accessor2 = columns.pop();
8787
first
@@ -177,7 +177,7 @@ fn get_ff_reader(
177177
/// Get all fast field reader or empty as default.
178178
///
179179
/// Is guaranteed to return at least one column.
180-
fn get_all_ff_reader(
180+
fn get_all_ff_reader_or_empty(
181181
reader: &SegmentReader,
182182
field_name: &str,
183183
allowed_column_types: Option<&[ColumnType]>,

src/aggregation/bucket/term_agg.rs

+43
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,12 @@ impl SegmentTermCollector {
428428
field_type: ColumnType,
429429
accessor_idx: usize,
430430
) -> crate::Result<Self> {
431+
if field_type == ColumnType::Bytes || field_type == ColumnType::Bool {
432+
return Err(TantivyError::InvalidArgument(format!(
433+
"terms aggregation is not supported for column type {:?}",
434+
field_type
435+
)));
436+
}
431437
let term_buckets = TermBuckets::default();
432438

433439
if let Some(custom_order) = req.order.as_ref() {
@@ -1500,4 +1506,41 @@ mod tests {
15001506

15011507
Ok(())
15021508
}
1509+
1510+
#[test]
1511+
fn terms_aggregation_bytes() -> crate::Result<()> {
1512+
let mut schema_builder = Schema::builder();
1513+
let bytes_field = schema_builder.add_bytes_field("bytes", FAST);
1514+
let index = Index::create_in_ram(schema_builder.build());
1515+
{
1516+
let mut index_writer = index.writer_with_num_threads(1, 20_000_000)?;
1517+
index_writer.set_merge_policy(Box::new(NoMergePolicy));
1518+
index_writer.add_document(doc!(
1519+
bytes_field => vec![1,2,3],
1520+
))?;
1521+
index_writer.commit()?;
1522+
}
1523+
1524+
let agg_req: Aggregations = serde_json::from_value(json!({
1525+
"my_texts": {
1526+
"terms": {
1527+
"field": "bytes"
1528+
},
1529+
}
1530+
}))
1531+
.unwrap();
1532+
1533+
let res = exec_request_with_query(agg_req, &index, None)?;
1534+
1535+
// TODO: Returning an error would be better instead of an empty result, since this is not a
1536+
// JSON field
1537+
assert_eq!(
1538+
res["my_texts"]["buckets"][0]["key"],
1539+
serde_json::Value::Null
1540+
);
1541+
assert_eq!(res["my_texts"]["sum_other_doc_count"], 0);
1542+
assert_eq!(res["my_texts"]["doc_count_error_upper_bound"], 0);
1543+
1544+
Ok(())
1545+
}
15031546
}

0 commit comments

Comments
 (0)