Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: function output field should not be inserted as dynamic field #380

Merged
merged 2 commits into from
Nov 26, 2024
Merged

fix: function output field should not be inserted as dynamic field #380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a796b05
fix: function output field should not be inserted as dynamic field
shanghaikid Nov 26, 2024
807fb5b
remove console
shanghaikid Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 42 additions & 28 deletions milvus/grpc/Data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,27 @@ export class Data extends Collection {
// Tip: The field data sequence needs to be set same as `collectionInfo.schema.fields`.
// If primarykey is set `autoid = true`, you cannot insert the data.
// and if function field is set, you need to ignore the field value in the data.
const functionOutputFields: string[] = [];
const fieldMap = new Map<string, _Field>(
collectionInfo.schema.fields
.filter(v => !v.is_primary_key || !v.autoID)
.filter(v => !v.is_function_output)
.map(v => [
v.name,
{
name: v.name,
type: v.data_type, // milvus return string here
elementType: v.element_type,
dim: Number(findKeyValue(v.type_params, 'dim')),
data: [], // values container
nullable: v.nullable,
default_value: v.default_value,
},
])
collectionInfo.schema.fields.reduce((acc, v) => {
if (v.is_function_output) {
functionOutputFields.push(v.name); // ignore function field
} else if (!v.is_primary_key || !v.autoID) {
acc.push([
v.name,
{
name: v.name,
type: v.data_type, // milvus return string here
elementType: v.element_type,
dim: Number(findKeyValue(v.type_params, 'dim')),
data: [], // values container
nullable: v.nullable,
default_value: v.default_value,
},
]);
}
return acc;
}, [] as [string, _Field][])
);

// dynamic field is enabled, create $meta field
Expand All @@ -179,7 +184,12 @@ export class Data extends Collection {
data.fields_data.forEach((rowData, rowIndex) => {
// if support dynamic field, all field not in the schema would be grouped to a dynamic field
rowData = isDynamic
? buildDynamicRow(rowData, fieldMap, DEFAULT_DYNAMIC_FIELD)
? buildDynamicRow(
rowData,
fieldMap,
DEFAULT_DYNAMIC_FIELD,
functionOutputFields
)
: rowData;

// get each fieldname from the row object
Expand Down Expand Up @@ -230,10 +240,18 @@ export class Data extends Collection {
const dataKey = getDataKey(type);
const elementType = DataTypeMap[field.elementType!];
const elementTypeKey = getDataKey(elementType);
const valid_data = getValidDataArray(
field.data,
data.fields_data?.length!
);

// check if need valid data
const needValidData =
key !== 'vectors' &&
(field.nullable === true ||
(typeof field.default_value !== 'undefined' &&
field.default_value !== null));

// get valid data
const valid_data = needValidData
? getValidDataArray(field.data, data.fields_data?.length!)
: [];

// build key value
let keyValue;
Expand Down Expand Up @@ -295,18 +313,12 @@ export class Data extends Collection {
break;
}

const needValidData =
key !== 'vectors' &&
(field.nullable === true ||
(typeof field.default_value !== 'undefined' &&
field.default_value !== null));

return {
type,
field_name: field.name,
is_dynamic: field.name === DEFAULT_DYNAMIC_FIELD,
[key]: keyValue,
valid_data: needValidData ? valid_data : [],
valid_data: valid_data,
};
});

Expand Down Expand Up @@ -478,7 +490,9 @@ export class Data extends Collection {
describeCollectionRequest.db_name = data.db_name;
}

const collectionInfo = await this.describeCollection(describeCollectionRequest);
const collectionInfo = await this.describeCollection(
describeCollectionRequest
);

// build search params
const { request, nq, round_decimal, isHybridSearch } = buildSearchRequest(
Expand Down
11 changes: 7 additions & 4 deletions milvus/utils/Format.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ export const formatDescribedCol = (
export const buildDynamicRow = (
rowData: RowData,
fieldMap: Map<string, _Field>,
dynamicFieldName: string
dynamicFieldName: string,
functionOutputFields: string[]
) => {
const originRow = cloneObj(rowData);

Expand All @@ -443,9 +444,11 @@ export const buildDynamicRow = (
// if the key is in the fieldMap, add it to the non-dynamic fields
row[key] = originRow[key];
} else {
const obj: JSON = row[dynamicFieldName] as JSON;
// otherwise, add it to the dynamic field
obj[key] = originRow[key];
if (!functionOutputFields.includes(key)) {
const obj: JSON = row[dynamicFieldName] as JSON;
// otherwise, add it to the dynamic field
obj[key] = originRow[key];
}
}
}

Expand Down
4 changes: 1 addition & 3 deletions test/grpc/FullTextSearch.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
dynamicFields,
} from '../tools';

const milvusClient = new MilvusClient({ address: IP, logLevel: 'debug' });
const milvusClient = new MilvusClient({ address: IP, logLevel: 'info' });
const COLLECTION = GENERATE_NAME();
const dbParam = {
db_name: 'FullTextSearch',
Expand Down Expand Up @@ -252,8 +252,6 @@ describe(`FulltextSearch API`, () => {
consistency_level: ConsistencyLevelEnum.Strong,
});

console.dir(search3, { depth: null });

expect(search3.status.error_code).toEqual(ErrorCode.SUCCESS);
});
});
18 changes: 12 additions & 6 deletions test/utils/Format.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,15 +488,21 @@ describe('utils/format', () => {
const data = {};
const fieldsDataMap = new Map();
const dynamicField = 'dynamic';
const result = buildDynamicRow(data, fieldsDataMap, dynamicField);
const result = buildDynamicRow(data, fieldsDataMap, dynamicField, []);
expect(result).toEqual({});
});

it('should return an object with dynamicField key when all data contains keys not in fieldsDataMap', () => {
const data = { key: 'value' };
const data = { key: 'value', key2: 'value2' };
const fieldsDataMap = new Map();
const dynamicField = 'dynamic';
const result = buildDynamicRow(data, fieldsDataMap, dynamicField);
const ignoreFields = ['key2'];
const result = buildDynamicRow(
data,
fieldsDataMap,
dynamicField,
ignoreFields
);
expect(result).toEqual({ [dynamicField]: { key: 'value' } });
});

Expand All @@ -513,7 +519,7 @@ describe('utils/format', () => {
],
]);
const dynamicField = 'dynamic';
const result = buildDynamicRow(data, fieldsDataMap, dynamicField);
const result = buildDynamicRow(data, fieldsDataMap, dynamicField, []);
expect(result).toEqual({
key1: 'value1',
[dynamicField]: { key2: 'value2' },
Expand Down Expand Up @@ -541,7 +547,7 @@ describe('utils/format', () => {
],
]);
const dynamicField = 'dynamic';
const result = buildDynamicRow(data, fieldsDataMap, dynamicField);
const result = buildDynamicRow(data, fieldsDataMap, dynamicField, []);
expect(result).toEqual({
[dynamicField]: {},
key1: 'value1',
Expand All @@ -562,7 +568,7 @@ describe('utils/format', () => {
],
]);
const dynamicField = 'dynamic';
const result = buildDynamicRow(data, fieldsDataMap, dynamicField);
const result = buildDynamicRow(data, fieldsDataMap, dynamicField, []);
expect(result).toEqual({
key1: 'value1',
[dynamicField]: { key2: 'value2' },
Expand Down
Loading