From 85c3aeb8c04cc7c19cda81ce98294b5ea98edaa3 Mon Sep 17 00:00:00 2001 From: Buqian Zheng Date: Fri, 17 Jul 2026 11:22:40 +0800 Subject: [PATCH] fix: honor scalar index policy for JSON string predicates (#51467) issue: https://github.com/milvus-io/milvus/issues/51466 ## What changed `PhyUnaryRangeFilterExpr::DetermineExecPath()` previously used a JSON-specific hard-coded denylist for string predicates: - `Match` - `PostfixMatch` - `InnerMatch` This bypassed the concrete scalar index's `ShouldUseOp()` policy. As a result: - indexes capable of evaluating these predicates, such as `STL_SORT`, were forced to scan and parse raw JSON; - JSON string predicates could use a different execution policy from ordinary VARCHAR predicates; - `RegexMatch` remained on the index path even for indexes whose planner policy explicitly preferred raw-data execution. --------- Signed-off-by: Buqian Zheng --- .../src/exec/expression/ExprJsonIndexTest.cpp | 106 ++++++++++++++++++ .../core/src/exec/expression/UnaryExpr.cpp | 6 +- 2 files changed, 108 insertions(+), 4 deletions(-) diff --git a/internal/core/src/exec/expression/ExprJsonIndexTest.cpp b/internal/core/src/exec/expression/ExprJsonIndexTest.cpp index 3eb911fdf7..b70668d54b 100644 --- a/internal/core/src/exec/expression/ExprJsonIndexTest.cpp +++ b/internal/core/src/exec/expression/ExprJsonIndexTest.cpp @@ -214,6 +214,112 @@ TYPED_TEST(JsonIndexTestFixture, TestJsonIndexUnaryExpr) { EXPECT_EQ(final.count(), N - expect_count); } +TEST(JsonIndexTest, JsonSortLikeUsesIndexWithoutRawJson) { + auto schema = std::make_shared(); + auto json_fid = schema->AddDebugField("json", DataType::JSON); + auto seg = CreateSealedSegment(schema); + + auto file_manager_ctx = storage::FileManagerContext(); + file_manager_ctx.fieldDataMeta.field_schema.set_data_type( + proto::schema::JSON); + file_manager_ctx.fieldDataMeta.field_schema.set_fieldid(json_fid.get()); + file_manager_ctx.fieldDataMeta.field_id = json_fid.get(); + + auto json_index = index::IndexFactory::GetInstance().CreateJsonIndex( + index::CreateIndexInfo{ + .index_type = index::ASCENDING_SORT, + .json_cast_type = JsonCastType::FromString("VARCHAR"), + .json_path = "/s", + }, + file_manager_ctx); + + const std::vector json_strs = { + R"({"s": "alpha"})", + R"({"s": "alphabet"})", + R"({"s": "beta"})", + R"({"s": "theta"})", + R"({"s": "alpha"})", + R"({"other": "alpha"})", + R"({"s": null})", + R"({"s": 42})", + }; + + // Load only the system row IDs so the sealed segment has its production + // row count. The raw JSON field remains deliberately unloaded. + auto cm = milvus::storage::RemoteChunkManagerSingleton::GetInstance() + .GetRemoteChunkManager(); + auto cm_w = ChunkManagerWrapper(cm); + std::vector row_ids(json_strs.size()); + for (size_t i = 0; i < row_ids.size(); ++i) { + row_ids[i] = i; + } + auto row_id_field_data = + storage::CreateFieldData(DataType::INT64, DataType::NONE, false); + row_id_field_data->FillFieldData(row_ids.data(), row_ids.size()); + auto row_id_load_info = PrepareSingleFieldInsertBinlog( + 1, 1, 1, RowFieldID.get(), {row_id_field_data}, cm); + seg->LoadFieldData(row_id_load_info); + + auto json_field = + std::make_shared>(DataType::JSON, false); + std::vector jsons; + jsons.reserve(json_strs.size()); + for (const auto& json : json_strs) { + jsons.emplace_back(simdjson::padded_string(json)); + } + json_field->add_json_data(jsons); + auto scalar_index = + dynamic_cast*>(json_index.get()); + ASSERT_NE(scalar_index, nullptr); + scalar_index->BuildWithFieldData({json_field}); + + segcore::LoadIndexInfo load_index_info; + load_index_info.field_id = json_fid.get(); + load_index_info.field_type = DataType::JSON; + load_index_info.index_params = {{JSON_PATH, "/s"}, + {JSON_CAST_TYPE, "VARCHAR"}}; + load_index_info.cache_index = + CreateTestCacheIndex("json_sort_like", std::move(json_index)); + seg->LoadIndex(load_index_info); + ASSERT_FALSE(seg->HasFieldData(json_fid)); + + // Deliberately do not load raw JSON field data. These predicates must be + // executable from the path index alone in lazy-load scenarios. + const std::vector< + std::tuple>> + test_cases = { + {proto::plan::OpType::InnerMatch, "pha", {0, 1, 4}}, + {proto::plan::OpType::PostfixMatch, "ta", {2, 3}}, + {proto::plan::OpType::Match, "a_ph%", {0, 1, 4}}, + }; + + for (const auto& [op, pattern, matched_rows] : test_cases) { + proto::plan::GenericValue value; + value.set_string_val(pattern); + auto unary_expr = std::make_shared( + expr::ColumnInfo(json_fid, DataType::JSON, {"s"}), + op, + value, + std::vector()); + auto plan = std::make_shared(DEFAULT_PLANNODE_ID, + unary_expr); + auto result = milvus::test::gen_filter_res( + plan.get(), seg.get(), json_strs.size(), MAX_TIMESTAMP); + TargetBitmapView result_view(result->GetRawData(), result->size()); + TargetBitmapView valid_view(result->GetValidRawData(), result->size()); + + std::vector expected(json_strs.size(), false); + for (auto row : matched_rows) { + expected[row] = true; + } + for (size_t i = 0; i < json_strs.size(); ++i) { + EXPECT_EQ(result_view[i], expected[i]) + << "op " << op << ", row " << i; + EXPECT_EQ(valid_view[i], i < 5) << "op " << op << ", row " << i; + } + } +} + TEST(JsonIndexTest, EmptyJsonInIsDeterministicForEveryRow) { auto schema = std::make_shared(); schema->AddDebugField( diff --git a/internal/core/src/exec/expression/UnaryExpr.cpp b/internal/core/src/exec/expression/UnaryExpr.cpp index c2594ec56e..7ac57ca5e9 100644 --- a/internal/core/src/exec/expression/UnaryExpr.cpp +++ b/internal/core/src/exec/expression/UnaryExpr.cpp @@ -2043,10 +2043,8 @@ PhyUnaryRangeFilterExpr::DetermineExecPath() { switch (val_type) { case DataType::STRING: case DataType::VARCHAR: - can_use = - expr_->op_type_ != proto::plan::OpType::Match && - expr_->op_type_ != proto::plan::OpType::PostfixMatch && - expr_->op_type_ != proto::plan::OpType::InnerMatch; + can_use = SegmentExpr::CanUseIndexForOp( + expr_->op_type_); break; default: can_use = true;