diff --git a/src/promql/src/functions/quantile.rs b/src/promql/src/functions/quantile.rs index 79acdf5ca5..bd18621f63 100644 --- a/src/promql/src/functions/quantile.rs +++ b/src/promql/src/functions/quantile.rs @@ -132,7 +132,7 @@ impl QuantileOverTime { value_len, &mut samples, ); - match quantile_with_scratch(window, quantile, &mut scratch) { + match window_quantile(window, quantile, &mut scratch) { Some(value) => result_builder.append_value(value), None => result_builder.append_null(), } @@ -183,7 +183,7 @@ impl QuantileOverTime { value_len, &mut samples, ); - match quantile_with_scratch(window, quantile, &mut scratch) { + match window_quantile(window, quantile, &mut scratch) { Some(value) => result_builder.append_value(value), None => result_builder.append_null(), } @@ -218,6 +218,18 @@ fn window_samples<'a>( samples } +/// Quantile of one range window, or `None` when the window holds no sample. +/// +/// Prometheus returns an empty vector for a range without float samples rather than the NaN +/// that [`quantile_impl`] yields for an empty slice, so the emptiness check belongs here and +/// not in the shared kernel. +fn window_quantile(values: &[f64], quantile: f64, scratch: &mut Vec) -> Option { + if values.is_empty() { + return None; + } + quantile_with_scratch(values, quantile, scratch) +} + /// Refer to pub(crate) fn quantile_impl(values: &[f64], quantile: f64) -> Option { let mut scratch = Vec::new(); @@ -337,8 +349,34 @@ mod tests { let output = extract_array(&QuantileOverTime::quantile_over_time(&input).unwrap()).unwrap(); let output = output.as_any().downcast_ref::().unwrap(); - assert_eq!(output.value(0), 2.5); - assert!(output.value(1).is_nan()); - assert!(output.value(2).is_nan()); + assert_eq!( + output.iter().collect::>(), + vec![Some(2.5), None, None] + ); + } + + #[test] + fn quantile_over_time_keeps_nan_for_an_invalid_quantile() { + let ts_array = Arc::new(TimestampMillisecondArray::from_iter_values([0i64, 1000])); + let values_array = Arc::new(Float64Array::from_iter_values([1.0, 4.0])); + let ranges = [(0, 2)]; + + let input = vec![ + ColumnarValue::Array(Arc::new( + RangeArray::from_ranges(ts_array, ranges) + .unwrap() + .into_dict(), + )), + ColumnarValue::Array(Arc::new( + RangeArray::from_ranges(values_array, ranges) + .unwrap() + .into_dict(), + )), + ColumnarValue::Scalar(ScalarValue::Float64(None)), + ]; + let output = extract_array(&QuantileOverTime::quantile_over_time(&input).unwrap()).unwrap(); + let output = output.as_any().downcast_ref::().unwrap(); + + assert!(output.value(0).is_nan()); } } diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index 7f1d5d35fb..e8b0432cbf 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -2142,8 +2142,6 @@ impl PromPlanner { ), }) }; - let preserve_any_value = - Self::field_columns_are_alternative_samples(input.schema(), &self.ctx.field_columns); let (mut func_exprs, new_tags) = self.create_function_expr( func, args.literals.clone(), @@ -2158,10 +2156,14 @@ impl PromPlanner { func_exprs.push(tsid_col); } + // Each field column is an independent series, so a row survives as long as any field + // produced a sample and the others stay NULL, matching what a selector emits. Requiring + // every field to be non-NULL would drop one field's samples because another field has + // none in that window. let builder = LogicalPlanBuilder::from(input) .project(func_exprs) .context(DataFusionPlanningSnafu)? - .filter(self.create_empty_values_filter_expr(preserve_any_value)?) + .filter(self.create_empty_values_filter_expr(true)?) .context(DataFusionPlanningSnafu)?; let builder = match func.name { diff --git a/tests/cases/standalone/common/promql/null_samples.result b/tests/cases/standalone/common/promql/null_samples.result index 5968cf4d1f..190815103f 100644 --- a/tests/cases/standalone/common/promql/null_samples.result +++ b/tests/cases/standalone/common/promql/null_samples.result @@ -150,6 +150,12 @@ TQL EVAL (1, 1, '1s') changes(sparse_samples[1s]); ++ ++ +-- Prometheus returns an empty vector for a range without samples, not NaN. +TQL EVAL (1, 1, '1s') quantile_over_time(0.5, sparse_samples[1s]); + +++ +++ + DROP TABLE sparse_samples; Affected Rows: 0 @@ -182,6 +188,26 @@ TQL EVAL (3, 3, '1s') rate(multi_field[4s]); | 1970-01-01T00:00:03 | 1.0 | 10.0 | a | +---------------------+---------------------------------------+---------------------------------------+------+ +-- f1 has two samples in this window and f2 only one, so f1 keeps its result while f2 is NULL. +-- SQLNESS SORT_RESULT 3 1 +TQL EVAL (1, 1, '1s') rate(multi_field[4s]); + ++---------------------+---------------------------------------+---------------------------------------+------+ +| ts | prom_rate(ts_range,f1,ts,Int64(4000)) | prom_rate(ts_range,f2,ts,Int64(4000)) | host | ++---------------------+---------------------------------------+---------------------------------------+------+ +| 1970-01-01T00:00:01 | 0.5 | | a | ++---------------------+---------------------------------------+---------------------------------------+------+ + +-- A selector emits the same shape: the row stays, the field without a sample is NULL. +-- SQLNESS SORT_RESULT 3 1 +TQL EVAL (1, 1, '1s') multi_field; + ++---------------------+------+-----+----+ +| ts | host | f1 | f2 | ++---------------------+------+-----+----+ +| 1970-01-01T00:00:01 | a | 2.0 | | ++---------------------+------+-----+----+ + DROP TABLE multi_field; Affected Rows: 0 diff --git a/tests/cases/standalone/common/promql/null_samples.sql b/tests/cases/standalone/common/promql/null_samples.sql index 41f7f34848..beceddfe87 100644 --- a/tests/cases/standalone/common/promql/null_samples.sql +++ b/tests/cases/standalone/common/promql/null_samples.sql @@ -59,6 +59,9 @@ TQL EVAL (1, 1, '1s') rate(sparse_samples[1s]); TQL EVAL (1, 1, '1s') changes(sparse_samples[1s]); +-- Prometheus returns an empty vector for a range without samples, not NaN. +TQL EVAL (1, 1, '1s') quantile_over_time(0.5, sparse_samples[1s]); + DROP TABLE sparse_samples; CREATE TABLE multi_field ( @@ -79,4 +82,12 @@ INSERT INTO multi_field VALUES -- SQLNESS SORT_RESULT 3 1 TQL EVAL (3, 3, '1s') rate(multi_field[4s]); +-- f1 has two samples in this window and f2 only one, so f1 keeps its result while f2 is NULL. +-- SQLNESS SORT_RESULT 3 1 +TQL EVAL (1, 1, '1s') rate(multi_field[4s]); + +-- A selector emits the same shape: the row stays, the field without a sample is NULL. +-- SQLNESS SORT_RESULT 3 1 +TQL EVAL (1, 1, '1s') multi_field; + DROP TABLE multi_field;