update delete api, comment, types, test (#262)

* update delete api, comment, types, test Signed-off-by: ryjiang <[email protected]> * fix build Signed-off-by: ryjiang <[email protected]> * add more test Signed-off-by: ryjiang <[email protected]> --------- Signed-off-by: ryjiang <[email protected]>
milvus-io · Dec 15, 2023 · dd8e4c3 · dd8e4c3
1 parent f83d438
commit dd8e4c3
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 28 deletions.
diff --git a/milvus/grpc/Data.ts b/milvus/grpc/Data.ts
@@ -32,6 +32,8 @@ import {
   QueryReq,
   GetReq,
   DeleteReq,
+  DeleteByIdsReq,
+  DeleteByFilterReq,
   QueryRes,
   SearchReq,
   SearchRes,
@@ -326,9 +328,9 @@ export class Data extends Collection {
    *  | :--- | :-- | :-- |
    *  | collection_name | String | Collection name |
    *  | partition_name(optional)| String | Partition name |
-   *  | ids | String[] or Number[] | ids to delete |
+   *  | ids | string[] or Number[] | ids to delete |
+   *  | filter| string | filter expression, filter takes precedence over ids |
    *  | timeout? | number | An optional duration of time in millisecond to allow for the RPC. If it is set to undefined, the client keeps waiting until the server responds or error occurs. Default is undefined |
-
    *
    * @returns
    * | Property | Description |
@@ -340,9 +342,16 @@ export class Data extends Collection {
    * #### Example
    *
    * ```
-   *  new milvusClient(MILUVS_ADDRESS).deleteEntities({
+   *  new milvusClient(MILUVS_ADDRESS).delete({
    *    collection_name: COLLECTION_NAME,
-   *    expr: 'id in [1,2,3,4]'
+   *    filter: 'id in [1,2,3,4]'
+   *  });
+   * ```
+   *
+   * ```
+   *  new milvusClient(MILUVS_ADDRESS).delete({
+   *    collection_name: COLLECTION_NAME,
+   *    ids: [1,2,3,4]
    *  });
    * ```
    */
@@ -351,18 +360,23 @@ export class Data extends Collection {
       throw new Error(ERROR_REASONS.COLLECTION_NAME_IS_REQUIRED);
     }
 
-    if (!data.ids || data.ids.length === 0) {
-      throw new Error(ERROR_REASONS.IDS_REQUIRED);
-    }
-
     const pkField = await this.getPkFieldName(data);
     const pkFieldType = await this.getPkFieldType(data);
 
+    let expr = '';
+
     // generate expr by different type of pk
-    const expr =
-      DataTypeMap[pkFieldType] === DataType.VarChar
-        ? `${pkField} in ["${data.ids.join('","')}"]`
-        : `${pkField} in [${data.ids.join(',')}]`;
+    if ((data as DeleteByIdsReq).ids) {
+      expr =
+        DataTypeMap[pkFieldType] === DataType.VarChar
+          ? `${pkField} in ["${(data as DeleteByIdsReq).ids.join('","')}"]`
+          : `${pkField} in [${(data as DeleteByIdsReq).ids.join(',')}]`;
+    }
+
+    // if filter exist use filter;
+    if ((data as DeleteByFilterReq).filter) {
+      expr = (data as DeleteByFilterReq).filter;
+    }
     const req = { ...data, expr };
     return this.deleteEntities(req);
   }

diff --git a/milvus/orm/Collection.ts b/milvus/orm/Collection.ts
@@ -9,6 +9,8 @@ import {
   DataTypeMap,
   DataType,
   findKeyValue,
+  DeleteByIdsReq,
+  DeleteByFilterReq,
 } from '..';
 
 interface collectionProps {
@@ -144,11 +146,20 @@ export class Collection {
   }
 
   // Deletes an entity from the collection.
-  async delete(data: Omit<DeleteReq, 'collection_name'>) {
-    // Create a request object to delete the entity.
-    const deleteReq = cloneObj(data) as DeleteReq;
-
-    deleteReq.collection_name = this.name;
+  async delete(
+    data:
+      | { ids: string[] | number[]; partition_name?: string }
+      | { filter: string; partition_name?: string }
+  ) {
+    let deleteReq: DeleteReq;
+
+    if ('ids' in data) {
+      deleteReq = { ...data, collection_name: this.name } as DeleteByIdsReq;
+    } else if ('filter' in data) {
+      deleteReq = { ...data, collection_name: this.name } as DeleteByFilterReq;
+    } else {
+      throw new Error('Invalid delete request');
+    }
 
     return await this.#client.delete(deleteReq);
   }

diff --git a/milvus/types/Data.ts b/milvus/types/Data.ts
@@ -84,11 +84,18 @@ export interface DeleteEntitiesReq extends collectionNameReq {
   partition_name?: string;
 }
 
-export interface DeleteReq extends collectionNameReq {
+export interface DeleteByIdsReq extends collectionNameReq {
   ids: string[] | number[];
   partition_name?: string;
 }
 
+export interface DeleteByFilterReq extends collectionNameReq {
+  filter: string;
+  partition_name?: string;
+}
+
+export type DeleteReq = DeleteByIdsReq | DeleteByFilterReq;
+
 export interface CalcDistanceReq extends GrpcTimeOut {
   op_left: any;
   op_right: any;

diff --git a/test/grpc/Data.spec.ts b/test/grpc/Data.spec.ts
@@ -520,16 +520,6 @@ describe(`Data.API`, () => {
     expect(res.data.length).toEqual(0);
   });
 
-  it(`delete without ids should throw error`, async () => {
-    try {
-      await milvusClient.delete({
-        collection_name: COLLECTION_NAME,
-      } as any);
-    } catch (error) {
-      expect(error.message).toEqual(ERROR_REASONS.IDS_REQUIRED);
-    }
-  });
-
   it(`delete by ids should success`, async () => {
     const res = await milvusClient.delete({
       collection_name: COLLECTION_NAME,
@@ -539,6 +529,15 @@ describe(`Data.API`, () => {
     expect(res.status.error_code).toEqual(ErrorCode.SUCCESS);
   });
 
+  it(`delete by filter should success`, async () => {
+    const res = await milvusClient.delete({
+      collection_name: COLLECTION_NAME,
+      filter: 'id < 5',
+    });
+
+    expect(res.status.error_code).toEqual(ErrorCode.SUCCESS);
+  });
+
   it(`Get metrics should throw GET_METRIC_CHECK_PARAMS`, async () => {
     try {
       await milvusClient.getMetric({} as any);

diff --git a/test/grpc/Orm.spec.ts b/test/grpc/Orm.spec.ts
@@ -188,6 +188,18 @@ describe(`ORM Client API`, () => {
     });
 
     expect(deleteRes.status.error_code).toEqual(ErrorCode.SUCCESS);
+
+    // delete
+    const delete2Res = await collection.delete({
+      filter: 'id < 5',
+    });
+    expect(delete2Res.status.error_code).toEqual(ErrorCode.SUCCESS);
+
+    try {
+      await collection.delete({} as any);
+    } catch (error) {
+      expect(error.message).toEqual(`Invalid delete request`);
+    }
   });
 
   it(`index successfully`, async () => {