Skip to content

Commit c2e39e0

Browse files
[query-engine] Slice expression bug fix (open-telemetry#2636)
## Changes * Switch `Slice` expression (arrays and strings) to use `Slice(source, start, [length])` instead of `Slice(source, start_inclusive, [end_exclusive])` to match KQL [substring](https://learn.microsoft.com/kusto/query/substring-function?view=azure-data-explorer) behavior. /cc @albertlockett @drewrelmas --------- Co-authored-by: albertlockett <a.lockett@f5.com>
1 parent caa1873 commit c2e39e0

4 files changed

Lines changed: 80 additions & 127 deletions

File tree

rust/experimental/query_engine/engine-recordset-otlp-bridge/tests/otlp_kql_recordset.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,8 @@ fn test_substring_function() {
564564

565565
run_test("substring(attributes['greeting'], 6)", "world");
566566
run_test("substring(attributes['greeting'], 0, 5)", "hello");
567+
run_test("substring(attributes['greeting'], 1, 4)", "ello");
568+
run_test("substring(attributes['greeting'], 0, 50)", "hello world");
567569
}
568570

569571
#[test]

rust/experimental/query_engine/engine-recordset/src/scalars/scalar_expressions.rs

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -254,19 +254,19 @@ where
254254
ScalarExpression::Slice(s) => {
255255
let inner_value = execute_scalar_expression(execution_context, s.get_source())?;
256256

257-
let range_start_inclusive = match s.get_range_start_inclusive() {
257+
let range_start_inclusive = match s.get_range_start() {
258258
Some(start) => SliceScalarExpression::validate_resolved_range_value(
259259
start.get_query_location(),
260260
"start",
261261
execute_scalar_expression(execution_context, start)?.to_value(),
262262
)?,
263263
None => 0,
264264
};
265-
let range_end_exclusive = match s.get_range_end_exclusive() {
266-
Some(end) => Some(SliceScalarExpression::validate_resolved_range_value(
267-
end.get_query_location(),
268-
"end",
269-
execute_scalar_expression(execution_context, end)?.to_value(),
265+
let length = match s.get_range_length() {
266+
Some(length) => Some(SliceScalarExpression::validate_resolved_range_value(
267+
length.get_query_location(),
268+
"length",
269+
execute_scalar_expression(execution_context, length)?.to_value(),
270270
)?),
271271
None => None,
272272
};
@@ -278,7 +278,7 @@ where
278278
"String",
279279
string_value.get_value().chars().count(),
280280
range_start_inclusive,
281-
range_end_exclusive,
281+
length,
282282
)?;
283283

284284
ResolvedValue::Slice(Slice::String(StringSlice::from_char_range(
@@ -294,7 +294,7 @@ where
294294
"Array",
295295
array_value.len(),
296296
range_start_inclusive,
297-
range_end_exclusive,
297+
length,
298298
)?;
299299

300300
ResolvedValue::Slice(Slice::Array(ArraySlice::new(
@@ -1572,7 +1572,7 @@ mod tests {
15721572
),
15731573
ExpressionError::ValidationFailure(
15741574
QueryLocation::new_fake(),
1575-
"Range end for a slice expression cannot be a negative value".into(),
1575+
"Range length for a slice expression cannot be a negative value".into(),
15761576
),
15771577
);
15781578

@@ -1587,7 +1587,7 @@ mod tests {
15871587
),
15881588
ExpressionError::TypeMismatch(
15891589
QueryLocation::new_fake(),
1590-
"Range end for a slice expression should be an integer type".into(),
1590+
"Range length for a slice expression should be an integer type".into(),
15911591
),
15921592
);
15931593
}
@@ -1673,7 +1673,7 @@ mod tests {
16731673
IntegerScalarExpression::new(QueryLocation::new_fake(), 1),
16741674
))),
16751675
),
1676-
Value::String(&StringValueStorage::new("".into())),
1676+
Value::String(&StringValueStorage::new("".into())),
16771677
);
16781678

16791679
run_test_success(
@@ -1687,7 +1687,7 @@ mod tests {
16871687
IntegerScalarExpression::new(QueryLocation::new_fake(), 2),
16881688
))),
16891689
),
1690-
Value::String(&StringValueStorage::new("".into())),
1690+
Value::String(&StringValueStorage::new("んに".into())),
16911691
);
16921692

16931693
run_test_success(
@@ -1719,32 +1719,15 @@ mod tests {
17191719
QueryLocation::new_fake(),
17201720
string_source.clone(),
17211721
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
1722-
IntegerScalarExpression::new(QueryLocation::new_fake(), 2),
1723-
))),
1724-
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
1725-
IntegerScalarExpression::new(QueryLocation::new_fake(), 1),
1722+
IntegerScalarExpression::new(QueryLocation::new_fake(), 5),
17261723
))),
1727-
),
1728-
ExpressionError::ValidationFailure(
1729-
QueryLocation::new_fake(),
1730-
"String slice index starts at '2' but ends at '1'".into(),
1731-
),
1732-
);
1733-
1734-
run_test_failure(
1735-
SliceScalarExpression::new(
1736-
QueryLocation::new_fake(),
1737-
string_source.clone(),
17381724
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
17391725
IntegerScalarExpression::new(QueryLocation::new_fake(), 1),
17401726
))),
1741-
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
1742-
IntegerScalarExpression::new(QueryLocation::new_fake(), 6),
1743-
))),
17441727
),
17451728
ExpressionError::ValidationFailure(
17461729
QueryLocation::new_fake(),
1747-
"String slice index ends at '6' which is beyond the length of '5'".into(),
1730+
"String slice index starts at '5' but target ends at index '4'".into(),
17481731
),
17491732
);
17501733
}
@@ -1856,7 +1839,7 @@ mod tests {
18561839
),
18571840
Value::Array(&ArrayScalarExpression::new(
18581841
QueryLocation::new_fake(),
1859-
array_values.clone().drain(1..1).collect(),
1842+
array_values.clone().drain(1..2).collect(),
18601843
)),
18611844
);
18621845

@@ -1873,7 +1856,7 @@ mod tests {
18731856
),
18741857
Value::Array(&ArrayScalarExpression::new(
18751858
QueryLocation::new_fake(),
1876-
array_values.clone().drain(1..2).collect(),
1859+
array_values.clone().drain(1..3).collect(),
18771860
)),
18781861
);
18791862

@@ -1912,32 +1895,15 @@ mod tests {
19121895
QueryLocation::new_fake(),
19131896
array_source.clone(),
19141897
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
1915-
IntegerScalarExpression::new(QueryLocation::new_fake(), 2),
1916-
))),
1917-
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
1918-
IntegerScalarExpression::new(QueryLocation::new_fake(), 1),
1898+
IntegerScalarExpression::new(QueryLocation::new_fake(), 5),
19191899
))),
1920-
),
1921-
ExpressionError::ValidationFailure(
1922-
QueryLocation::new_fake(),
1923-
"Array slice index starts at '2' but ends at '1'".into(),
1924-
),
1925-
);
1926-
1927-
run_test_failure(
1928-
SliceScalarExpression::new(
1929-
QueryLocation::new_fake(),
1930-
array_source.clone(),
19311900
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
19321901
IntegerScalarExpression::new(QueryLocation::new_fake(), 1),
19331902
))),
1934-
Some(ScalarExpression::Static(StaticScalarExpression::Integer(
1935-
IntegerScalarExpression::new(QueryLocation::new_fake(), 6),
1936-
))),
19371903
),
19381904
ExpressionError::ValidationFailure(
19391905
QueryLocation::new_fake(),
1940-
"Array slice index ends at '6' which is beyond the length of '5'".into(),
1906+
"Array slice index starts at '5' but target ends at index '4'".into(),
19411907
),
19421908
);
19431909
}

0 commit comments

Comments
 (0)