unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
milvus-io/milvus
mid7/10
query-optimizationerror-handlingtesting-strategy

fix: validate MATCH predicates are element-level [3.0]

Project: milvus-io/milvus | Language: C++ | Concepts: query-optimization, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/milvus-io/milvus/pull/51940

The problem

MATCH predicates in Milvus were not being strictly validated to ensure they operated at the element level. This could lead to incorrect query results or internal errors, especially when queries used out-of-range values or malformed predicate shapes on array/struct fields. The system needed to ensure that such predicates correctly handled element-level semantics and overflow cases, and that the results' sizes matched expectations.

Why it's non-trivial

A naive fix might only check for nulls or simply short-circuit on overflow, but would miss:

  • Properly aligning element-level batch processing with the row cursor, so that short-circuit logic (like skipping batches on overflow) doesn't desynchronize result validity.
  • Ensuring that the result bitmap size matches the number of elements processed, not just rows.
  • Handling both indexed and non-indexed cases, as well as various input forms (with or without offset vectors).
  • Correctly propagating validity and result bits for out-of-range predicates, especially in complex nested or array structures.

The approach

1. Strict validation of element-level predicate result sizes

In MatchExpr.cpp, the code now asserts that the result size of a MATCH predicate exactly matches the number of elements being processed:

+    AssertInfo(match_result_col_vec->size() == elem_count,
+               "Match predicate result size {} should equal element count {}",
+               match_result_col_vec->size(),
+               elem_count);

This ensures that any mismatch between the expected number of elements and the actual result triggers a clear error, preventing subtle bugs later in query processing.

2. Overflow handling for element-level predicates in batch processing

In both BinaryRangeExpr.cpp and UnaryExpr.cpp, the logic for handling batch overflow checks is updated to respect element-level semantics:

+        TargetBitmap valid_res;
+        if (expr_->column_.element_level_) {
+            // Element batches are derived from the row cursor so
+            // MoveCursor()-based short-circuiting stays aligned.
+            // Individual elements cannot be null; their containing row's
+            // validity is applied by the element consumer.
+            auto batch_size =
+                GetNextRealBatchSize(input, /*element_level=*/true);
+            valid_res = TargetBitmap(batch_size, true);
+            if (input == nullptr) {
+                MoveCursor();
+            }
+        } else if (input != nullptr) {
+            valid_res =
+                ProcessChunksForValidByOffsets<T>(UseIndexCursor(), *input);
+        } else {
+            valid_res = ProcessChunksForValid<T>(UseIndexCursor());
+        }

This block ensures that for element-level columns, batch sizes and validity are computed per-element, not per-row, and that cursor advancement stays in sync.

3. Consistent overflow result generation for all predicate types

In UnaryExpr.cpp, the code for creating the result bitmap in overflow cases is refactored to always respect element-level batching and validity:

+            auto make_overflow_result =
+                [this, input](bool match_value) -> ColumnVectorPtr {
+                TargetBitmap valid;
+                if (expr_->column_.element_level_) {
+                    // Element batches are derived from the row cursor so
+                    // MoveCursor()-based short-circuiting stays aligned.
+                    // Individual elements cannot be null; their containing
+                    // row's validity is applied by the element consumer.
+                    auto batch_size =
+                        GetNextRealBatchSize(input, /*element_level=*/true);
+                    valid = TargetBitmap(batch_size, true);
+                    if (input == nullptr) {
+                        MoveCursor();
+                    }
+                } else if (input != nullptr) {
+                    valid = ProcessChunksForValidByOffsets<T>(UseIndexCursor(),
+                                                              *input);
+                } else {
+                    valid = ProcessChunksForValid<T>(UseIndexCursor());
+                }
+
+                auto batch_size = valid.size();
+                TargetBitmap res(batch_size, match_value);
+                if (match_value) {
+                    res &= valid;
+                }
+                return std::make_shared<ColumnVector>(std::move(res),
+                                                      std::move(valid));
+            };

This closure is used to generate the correct result and validity bitmaps for all overflow cases, whether the predicate is equality, range, or not-equal, and ensures element-level alignment.

4. Comprehensive test coverage for edge cases

New and extended tests in MatchExprTest.cpp cover:

  • Out-of-range values for all integer types.
  • Both indexed and non-indexed data.
  • Offset vector (sparse) queries and batched queries.
  • Validation that overflow predicates return no rows or only valid rows as appropriate.

Example new test:

+TEST_P(SealedMatchExprIntTypeTest, MatchAnyOverflowShortcutReturnsNoRows) {
+    auto param = GetParam();
+    auto filter_expr =
+        "match_any(struct_array, $[sub_int] == " + OverflowLiteral() + ")";
+    auto result = ExecuteRetrieve(filter_expr);
+
+    ASSERT_NE(result, nullptr);
+    EXPECT_EQ(result->offset_size(), 0)
+        << param.type_name << " overflow equality should match no rows";
+}

This test ensures that an equality predicate with an out-of-range literal returns no matches, for all integer types.

From the review thread

buqian-zilliz:

[P2] Reject malformed templated range predicates

Checking only the column flag here lets a malformed templated comparison pass the new MATCH validation. For MATCH_ANY(sa, {k} + $[a] < $[b]), the left arithmetic node is marked templated, so handleCompare takes its generic left.expr.GetIsTemplate() branch even though it is not a ValueExpr; it builds a UnaryRangeExpr on $[b] with neither Value nor TemplateVariableName and leaves IsTemplate false. This branch accepts that node, FillExpressionValue skips it, and segcore later decodes the unset GenericValue and fails. Require a concrete value or template name before accepting a unary range, or reject or fix this unsupported comparison shape.

Takeaways

  • Always validate that result sizes match expectations for element-level operations.
    When processing arrays or nested structures, ensure that output bitmaps or vectors align with the actual number of elements, not just rows.

  • Handle overflow and out-of-range cases early and consistently.
    Short-circuit evaluation for predicates that cannot possibly match (e.g., value out of type range) should generate result and validity bitmaps that are correct for both element- and row-level queries.

  • Comprehensive tests should cover all edge cases, including overflow, batching, and sparse inputs.
    Tests should explicitly check that no rows are returned for impossible predicates and that validity is preserved in all scenarios.

  • Be wary of malformed or unsupported predicate shapes in query planning.
    Validation should reject or properly handle cases where a predicate is neither a concrete value nor a valid template, to prevent downstream errors.