From 4887bd0957ec07cee090e03b271916fd8aa4f672 Mon Sep 17 00:00:00 2001 From: Shaya Potter Date: Tue, 23 Jul 2024 16:37:59 +0300 Subject: [PATCH 1/4] small refactor per discussion with leibele --- packages/search/lib/commands/CREATE.spec.ts | 21 +++++++++++++++------ packages/search/lib/commands/index.ts | 18 ++++++++++-------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/search/lib/commands/CREATE.spec.ts b/packages/search/lib/commands/CREATE.spec.ts index 094ba4529f0..48f76ad497f 100644 --- a/packages/search/lib/commands/CREATE.spec.ts +++ b/packages/search/lib/commands/CREATE.spec.ts @@ -448,9 +448,7 @@ describe('CREATE', () => { transformArguments('index', { field: { type: SchemaFieldTypes.TEXT, - MISSING_VALUES: { - INDEXEMPTY: true - } + INDEXEMPTY: true } }), ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXEMPTY'] @@ -462,14 +460,25 @@ describe('CREATE', () => { transformArguments('index', { field: { type: SchemaFieldTypes.TEXT, - MISSING_VALUES: { - INDEXMISSING: true - } + INDEXMISSING: true } }), ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXMISSING'] ); }); + + it('with INDEXEMPTY and INDEXMISSING', () => { + assert.deepEqual( + transformArguments('index', { + field: { + type: SchemaFieldTypes.TEXT, + INDEXEMPTY: true, + INDEXMISSING: true + } + }), + ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXEMPTY', 'INDEXMISSING'] + ); + }); }); }); diff --git a/packages/search/lib/commands/index.ts b/packages/search/lib/commands/index.ts index 29dbd2480ce..210faa37ca5 100644 --- a/packages/search/lib/commands/index.ts +++ b/packages/search/lib/commands/index.ts @@ -190,8 +190,8 @@ export enum SchemaFieldTypes { } export interface MissingValues { - INDEXEMPTY?: boolean; - INDEXMISSING?: boolean; + INDEXEMPTY?: true; + INDEXMISSING?: true; } function pushMissingValues(args: RedisCommandArguments, missingValues?: MissingValues) { @@ -214,7 +214,7 @@ type CreateSchemaField< > = T | ({ type: T; AS?: string; - MISSING_VALUES?: MissingValues; + INDEXMISSING?: true; } & E); type CreateSchemaCommonField< @@ -240,6 +240,7 @@ type CreateSchemaTextField = CreateSchemaCommonField; type CreateSchemaNumericField = CreateSchemaCommonField; @@ -250,6 +251,7 @@ type CreateSchemaTagField = CreateSchemaCommonField; export enum VectorAlgorithms { @@ -333,13 +335,13 @@ export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema args.push('WITHSUFFIXTRIE'); } - pushMissingValues(args, fieldOptions.MISSING_VALUES); + pushMissingValues(args, fieldOptions); break; case SchemaFieldTypes.NUMERIC: case SchemaFieldTypes.GEO: - pushMissingValues(args, fieldOptions.MISSING_VALUES); + pushMissingValues(args, fieldOptions); break; case SchemaFieldTypes.TAG: @@ -355,7 +357,7 @@ export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema args.push('WITHSUFFIXTRIE'); } - pushMissingValues(args, fieldOptions.MISSING_VALUES); + pushMissingValues(args, fieldOptions); break; @@ -398,7 +400,7 @@ export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema } }); - pushMissingValues(args, fieldOptions.MISSING_VALUES); + pushMissingValues(args, fieldOptions); continue; // vector fields do not contain SORTABLE and NOINDEX options @@ -407,7 +409,7 @@ export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema args.push('COORD_SYSTEM', fieldOptions.COORD_SYSTEM); } - pushMissingValues(args, fieldOptions.MISSING_VALUES); + pushMissingValues(args, fieldOptions); continue; // geo shape fields do not contain SORTABLE and NOINDEX options } From 576bda25acafb4f5a21a6e0e4d4b0960ae7cc91f Mon Sep 17 00:00:00 2001 From: Shaya Potter Date: Tue, 23 Jul 2024 21:51:10 +0300 Subject: [PATCH 2/4] move true type to boolean type --- packages/search/lib/commands/index.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/search/lib/commands/index.ts b/packages/search/lib/commands/index.ts index 210faa37ca5..0a59a6eaa5e 100644 --- a/packages/search/lib/commands/index.ts +++ b/packages/search/lib/commands/index.ts @@ -190,8 +190,8 @@ export enum SchemaFieldTypes { } export interface MissingValues { - INDEXEMPTY?: true; - INDEXMISSING?: true; + INDEXEMPTY?: boolean; + INDEXMISSING?: boolean; } function pushMissingValues(args: RedisCommandArguments, missingValues?: MissingValues) { @@ -214,7 +214,7 @@ type CreateSchemaField< > = T | ({ type: T; AS?: string; - INDEXMISSING?: true; + INDEXMISSING?: boolean; } & E); type CreateSchemaCommonField< @@ -240,7 +240,7 @@ type CreateSchemaTextField = CreateSchemaCommonField; type CreateSchemaNumericField = CreateSchemaCommonField; @@ -251,7 +251,7 @@ type CreateSchemaTagField = CreateSchemaCommonField; export enum VectorAlgorithms { From 15580a3c602fd878a49e95cecf80fdd00226ab33 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Wed, 24 Jul 2024 13:14:35 -0400 Subject: [PATCH 3/4] fix geoshape to support NOINDEX & SORTABLE, clean code --- packages/search/lib/commands/CREATE.spec.ts | 75 ++++++++++----------- packages/search/lib/commands/index.ts | 69 +++++++++---------- 2 files changed, 67 insertions(+), 77 deletions(-) diff --git a/packages/search/lib/commands/CREATE.spec.ts b/packages/search/lib/commands/CREATE.spec.ts index 48f76ad497f..50c5c011c89 100644 --- a/packages/search/lib/commands/CREATE.spec.ts +++ b/packages/search/lib/commands/CREATE.spec.ts @@ -70,6 +70,18 @@ describe('CREATE', () => { ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'WITHSUFFIXTRIE'] ); }); + + it('with INDEXEMPTY', () => { + assert.deepEqual( + transformArguments('index', { + field: { + type: SchemaFieldTypes.TEXT, + INDEXEMPTY: true + } + }), + ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXEMPTY'] + ); + }); }); it('NUMERIC', () => { @@ -148,6 +160,18 @@ describe('CREATE', () => { ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TAG', 'WITHSUFFIXTRIE'] ); }); + + it('with INDEXEMPTY', () => { + assert.deepEqual( + transformArguments('index', { + field: { + type: SchemaFieldTypes.TAG, + INDEXEMPTY: true + } + }), + ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TAG', 'INDEXEMPTY'] + ); + }); }); describe('VECTOR', () => { @@ -282,6 +306,18 @@ describe('CREATE', () => { ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'NOINDEX'] ); }); + + it('with INDEXMISSING', () => { + assert.deepEqual( + transformArguments('index', { + field: { + type: SchemaFieldTypes.TEXT, + INDEXMISSING: true + } + }), + ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXMISSING'] + ); + }); }); }); @@ -441,45 +477,6 @@ describe('CREATE', () => { ); }); }); - - describe('Missing Values', () => { - it('with INDEXEMPTY', () => { - assert.deepEqual( - transformArguments('index', { - field: { - type: SchemaFieldTypes.TEXT, - INDEXEMPTY: true - } - }), - ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXEMPTY'] - ); - }); - - it('with INDEXMISSING', () => { - assert.deepEqual( - transformArguments('index', { - field: { - type: SchemaFieldTypes.TEXT, - INDEXMISSING: true - } - }), - ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXMISSING'] - ); - }); - - it('with INDEXEMPTY and INDEXMISSING', () => { - assert.deepEqual( - transformArguments('index', { - field: { - type: SchemaFieldTypes.TEXT, - INDEXEMPTY: true, - INDEXMISSING: true - } - }), - ['FT.CREATE', 'index', 'SCHEMA', 'field', 'TEXT', 'INDEXEMPTY', 'INDEXMISSING'] - ); - }); - }); }); testUtils.testWithClient('client.ft.create', async client => { diff --git a/packages/search/lib/commands/index.ts b/packages/search/lib/commands/index.ts index 0a59a6eaa5e..573a2a5d949 100644 --- a/packages/search/lib/commands/index.ts +++ b/packages/search/lib/commands/index.ts @@ -188,25 +188,6 @@ export enum SchemaFieldTypes { VECTOR = 'VECTOR', GEOSHAPE = 'GEOSHAPE' } - -export interface MissingValues { - INDEXEMPTY?: boolean; - INDEXMISSING?: boolean; -} - -function pushMissingValues(args: RedisCommandArguments, missingValues?: MissingValues) { - if (!missingValues) { - return; - } - - if (missingValues.INDEXEMPTY) { - args.push("INDEXEMPTY"); - } - - if (missingValues.INDEXMISSING) { - args.push("INDEXMISSING"); - } -} type CreateSchemaField< T extends SchemaFieldTypes, @@ -228,6 +209,20 @@ type CreateSchemaCommonField< } & E) >; +function pushCommonFieldArguments(args: RedisCommandArguments, fieldOptions: CreateSchemaCommonField) { + if (fieldOptions.SORTABLE) { + args.push('SORTABLE'); + + if (fieldOptions.SORTABLE === 'UNF') { + args.push('UNF'); + } + } + + if (fieldOptions.NOINDEX) { + args.push('NOINDEX'); + } +} + export enum SchemaTextFieldPhonetics { DM_EN = 'dm:en', DM_FR = 'dm:fr', @@ -299,7 +294,7 @@ export interface RediSearchSchema { CreateSchemaTagField | CreateSchemaFlatVectorField | CreateSchemaHNSWVectorField | - CreateSchemaGeoShapeField + CreateSchemaGeoShapeField; } export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema) { @@ -335,13 +330,17 @@ export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema args.push('WITHSUFFIXTRIE'); } - pushMissingValues(args, fieldOptions); + pushCommonFieldArguments(args, fieldOptions); + + if (fieldOptions.INDEXEMPTY) { + args.push('INDEXEMPTY'); + } break; case SchemaFieldTypes.NUMERIC: case SchemaFieldTypes.GEO: - pushMissingValues(args, fieldOptions); + pushCommonFieldArguments(args, fieldOptions); break; case SchemaFieldTypes.TAG: @@ -357,7 +356,11 @@ export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema args.push('WITHSUFFIXTRIE'); } - pushMissingValues(args, fieldOptions); + pushCommonFieldArguments(args, fieldOptions); + + if (fieldOptions.INDEXEMPTY) { + args.push('INDEXEMPTY'); + } break; @@ -400,30 +403,20 @@ export function pushSchema(args: RedisCommandArguments, schema: RediSearchSchema } }); - pushMissingValues(args, fieldOptions); - - continue; // vector fields do not contain SORTABLE and NOINDEX options + break; case SchemaFieldTypes.GEOSHAPE: if (fieldOptions.COORD_SYSTEM !== undefined) { args.push('COORD_SYSTEM', fieldOptions.COORD_SYSTEM); } - pushMissingValues(args, fieldOptions); - - continue; // geo shape fields do not contain SORTABLE and NOINDEX options - } + pushCommonFieldArguments(args, fieldOptions); - if (fieldOptions.SORTABLE) { - args.push('SORTABLE'); - - if (fieldOptions.SORTABLE === 'UNF') { - args.push('UNF'); - } + break; } - if (fieldOptions.NOINDEX) { - args.push('NOINDEX'); + if (fieldOptions.INDEXMISSING) { + args.push('INDEXMISSING'); } } } From 64d4bbaa8f2009ffc16366b54e644faebc32ef18 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Wed, 24 Jul 2024 15:06:23 -0400 Subject: [PATCH 4/4] fix for last commit --- packages/search/lib/commands/index.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/search/lib/commands/index.ts b/packages/search/lib/commands/index.ts index 573a2a5d949..f907e1999e6 100644 --- a/packages/search/lib/commands/index.ts +++ b/packages/search/lib/commands/index.ts @@ -198,18 +198,20 @@ type CreateSchemaField< INDEXMISSING?: boolean; } & E); +type CommonFieldArguments = { + SORTABLE?: boolean | 'UNF'; + NOINDEX?: boolean; +}; + type CreateSchemaCommonField< T extends SchemaFieldTypes, E = Record > = CreateSchemaField< T, - ({ - SORTABLE?: true | 'UNF'; - NOINDEX?: true; - } & E) + (CommonFieldArguments & E) >; -function pushCommonFieldArguments(args: RedisCommandArguments, fieldOptions: CreateSchemaCommonField) { +function pushCommonFieldArguments(args: RedisCommandArguments, fieldOptions: CommonFieldArguments) { if (fieldOptions.SORTABLE) { args.push('SORTABLE');