From deda656dccb4102bf1964383e02bb2984eb69f4b Mon Sep 17 00:00:00 2001 From: aoiasd <45024769+aoiasd@users.noreply.github.com> Date: Tue, 23 Apr 2024 17:45:27 +0800 Subject: [PATCH] enhance:[Cherry-pick] forbid delete with always true expression (#32472) (#32495) pr: https://github.com/milvus-io/milvus/pull/32472 Signed-off-by: aoiasd --- internal/proxy/task_delete.go | 6 +++++- internal/proxy/task_delete_test.go | 32 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/internal/proxy/task_delete.go b/internal/proxy/task_delete.go index fda28b7513..b5d454f4ff 100644 --- a/internal/proxy/task_delete.go +++ b/internal/proxy/task_delete.go @@ -300,11 +300,15 @@ func (dr *deleteRunner) Init(ctx context.Context) error { } func (dr *deleteRunner) Run(ctx context.Context) error { - plan, err := planparserv2.CreateRetrievePlan(dr.schema.CollectionSchema, dr.req.Expr) + plan, err := planparserv2.CreateRetrievePlan(dr.schema.CollectionSchema, dr.req.GetExpr()) if err != nil { return fmt.Errorf("failed to create expr plan, expr = %s", dr.req.GetExpr()) } + if planparserv2.IsAlwaysTruePlan(plan) { + return merr.WrapErrParameterInvalidMsg("delete plan can't be empty or always true : %s", dr.req.GetExpr()) + } + isSimple, pk, numRow := getPrimaryKeysFromPlan(dr.schema.CollectionSchema, plan) if isSimple { // if could get delete.primaryKeys from delete expr diff --git a/internal/proxy/task_delete_test.go b/internal/proxy/task_delete_test.go index ca40dcea58..769a01dd15 100644 --- a/internal/proxy/task_delete_test.go +++ b/internal/proxy/task_delete_test.go @@ -517,6 +517,38 @@ func TestDeleteRunner_Run(t *testing.T) { assert.Equal(t, int64(0), dr.result.DeleteCnt) }) + t.Run("delete with always true expression failed", func(t *testing.T) { + mockMgr := NewMockChannelsMgr(t) + lb := NewMockLBPolicy(t) + + dr := deleteRunner{ + chMgr: mockMgr, + schema: schema, + collectionID: collectionID, + partitionID: partitionID, + vChannels: channels, + tsoAllocatorIns: tsoAllocator, + idAllocator: idAllocator, + queue: queue.dmQueue, + lb: lb, + result: &milvuspb.MutationResult{ + Status: merr.Success(), + IDs: &schemapb.IDs{ + IdField: nil, + }, + }, + req: &milvuspb.DeleteRequest{ + CollectionName: collectionName, + PartitionName: partitionName, + DbName: dbName, + Expr: " ", + }, + } + + assert.Error(t, dr.Run(context.Background())) + assert.Equal(t, int64(0), dr.result.DeleteCnt) + }) + t.Run("complex delete query rpc failed", func(t *testing.T) { mockMgr := NewMockChannelsMgr(t) qn := mocks.NewMockQueryNodeClient(t)