Merge pull request #183 from weaviate/fix/merge-with-existing-in-coll…

…ection-update Fix/merge with existing in collection update
weaviate · Aug 6, 2024 · a5355e2 · a5355e2
2 parents b2dc636 + a356e68
commit a5355e2
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 106 deletions.
diff --git a/src/collections/config/classes.ts b/src/collections/config/classes.ts
@@ -38,7 +38,7 @@ export class MergeWithExisting {
     if (update.vectorizers !== undefined) {
       if (Array.isArray(update.vectorizers)) {
         current.vectorConfig = MergeWithExisting.vectors(current.vectorConfig, update.vectorizers);
-      } else if (supportsNamedVectors) {
+      } else if (supportsNamedVectors && current.vectorConfig !== undefined) {
         const updateVectorizers = {
           ...update.vectorizers,
           name: 'default',

diff --git a/src/collections/config/index.ts b/src/collections/config/index.ts
@@ -16,6 +16,8 @@ import {
   CollectionConfig,
   CollectionConfigUpdate,
   PQConfig,
+  QuantizerConfig,
+  SQConfig,
   VectorIndexConfig,
   VectorIndexConfigDynamic,
   VectorIndexConfigFlat,
@@ -163,12 +165,15 @@ export class VectorIndex {
 }
 
 export class Quantizer {
-  static isPQ(config?: PQConfig | BQConfig): config is PQConfig {
+  static isPQ(config?: QuantizerConfig): config is PQConfig {
     return config?.type === 'pq';
   }
-  static isBQ(config?: PQConfig | BQConfig): config is BQConfig {
+  static isBQ(config?: QuantizerConfig): config is BQConfig {
     return config?.type === 'bq';
   }
+  static isSQ(config?: QuantizerConfig): config is SQConfig {
+    return config?.type === 'sq';
+  }
 }
 
 export const configGuards = {

diff --git a/src/collections/config/integration.test.ts b/src/collections/config/integration.test.ts
@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 import { WeaviateUnsupportedFeatureError } from '../../errors.js';
-import weaviate, { WeaviateClient } from '../../index.js';
+import weaviate, { WeaviateClient, weaviateV2 } from '../../index.js';
 import { PropertyConfig, VectorIndexConfigDynamic, VectorIndexConfigHNSW } from './types/index.js';
 
 const fail = (msg: string) => {
@@ -526,71 +526,59 @@ describe('Testing of the collection.config namespace', () => {
     expect(config.multiTenancy.enabled).toEqual(true);
   });
 
-  // it('should be able update the config of a collection with legacy vectors', async () => {
-  //   const collectionName = 'TestCollectionConfigUpdateLegacyVectors';
-  //   const collection = await client.collections.create({
-  //     name: collectionName,
-  //     properties: [
-  //       {
-  //         name: 'testProp',
-  //         dataType: 'text',
-  //       },
-  //     ],
-  //     vectorizer: {
-  //       name: 'none',
-  //       config: {},
-  //     },
-  //   });
-  //   const config = await collection.config
-  //     .update({
-  //       vectorizers: weaviate.reconfigure.vectorIndex.hnsw({
-  //         quantizer: weaviate.reconfigure.vectorIndex.quantizer.pq(),
-  //         ef: 4,
-  //       }),
-  //     })
-  //     .then(() => collection.config.get());
-
-  //   expect(config.name).toEqual(collectionName);
-  //   expect(config.properties).toEqual<PropertyConfig[]>([
-  //     {
-  //       name: 'testProp',
-  //       dataType: 'text',
-  //       description: undefined,
-  //       indexSearchable: true,
-  //       indexFilterable: true,
-  //       indexInverted: false,
-  //       vectorizerConfig: undefined,
-  //       nestedProperties: undefined,
-  //       tokenization: 'word',
-  //     },
-  //   ]);
-  //   expect(config.generative).toBeUndefined();
-  //   expect(config.reranker).toBeUndefined();
-  //   expect(config.vectorizers.default.indexConfig).toEqual<VectorIndexConfigHNSW>({
-  //     skip: false,
-  //     cleanupIntervalSeconds: 300,
-  //     maxConnections: 64,
-  //     efConstruction: 128,
-  //     ef: 4,
-  //     dynamicEfMin: 100,
-  //     dynamicEfMax: 500,
-  //     dynamicEfFactor: 8,
-  //     vectorCacheMaxObjects: 1000000000000,
-  //     flatSearchCutoff: 40000,
-  //     distance: 'cosine',
-  //     quantizer: {
-  //       bitCompression: false,
-  //       segments: 0,
-  //       centroids: 256,
-  //       trainingLimit: 100000,
-  //       encoder: {
-  //         type: 'kmeans',
-  //         distribution: 'log-normal',
-  //       },
-  //       type: 'pq',
-  //     },
-  //   });
-  //   expect(config.vectorizers.default.indexType).toEqual('hnsw');
-  //   expect(config.vectorizers.default.vectorizer.name).toEqual('none');
-  // });
+  it('should be able update the config of a collection with legacy vectors', async () => {
+    const clientV2 = weaviateV2.client({
+      host: 'http://localhost:8080',
+    });
+    const collectionName = 'TestCollectionConfigUpdateLegacyVectors';
+    await clientV2.schema
+      .classCreator()
+      .withClass({
+        class: collectionName,
+        vectorizer: 'none',
+      })
+      .do();
+    const collection = client.collections.get(collectionName);
+    const config = await collection.config
+      .update({
+        vectorizers: weaviate.reconfigure.vectorizer.update({
+          vectorIndexConfig: weaviate.reconfigure.vectorIndex.hnsw({
+            quantizer: weaviate.reconfigure.vectorIndex.quantizer.pq(),
+            ef: 4,
+          }),
+        }),
+      })
+      .then(() => collection.config.get());
+
+    expect(config.name).toEqual(collectionName);
+    expect(config.generative).toBeUndefined();
+    expect(config.reranker).toBeUndefined();
+    expect(config.vectorizers.default.indexConfig).toEqual<VectorIndexConfigHNSW>({
+      skip: false,
+      cleanupIntervalSeconds: 300,
+      maxConnections: (await client.getWeaviateVersion().then((ver) => ver.isLowerThan(1, 26, 0))) ? 64 : 32,
+      efConstruction: 128,
+      ef: 4,
+      dynamicEfMin: 100,
+      dynamicEfMax: 500,
+      dynamicEfFactor: 8,
+      vectorCacheMaxObjects: 1000000000000,
+      flatSearchCutoff: 40000,
+      distance: 'cosine',
+      type: 'hnsw',
+      quantizer: {
+        bitCompression: false,
+        segments: 0,
+        centroids: 256,
+        trainingLimit: 100000,
+        encoder: {
+          type: 'kmeans',
+          distribution: 'log-normal',
+        },
+        type: 'pq',
+      },
+    });
+    expect(config.vectorizers.default.indexType).toEqual('hnsw');
+    expect(config.vectorizers.default.vectorizer.name).toEqual('none');
+  });
 });
diff --git a/src/collections/config/types/vectorIndex.ts b/src/collections/config/types/vectorIndex.ts
@@ -73,3 +73,5 @@ export type PQEncoderDistribution = 'log-normal' | 'normal';
 export type VectorIndexType = 'hnsw' | 'flat' | 'dynamic' | string;
 
 export type VectorIndexConfig = VectorIndexConfigHNSW | VectorIndexConfigFlat | VectorIndexConfigDynamic;
+
+export type QuantizerConfig = PQConfig | BQConfig | SQConfig;
diff --git a/src/collections/configure/parsing.ts b/src/collections/configure/parsing.ts
@@ -39,21 +39,3 @@ export class QuantizerGuards {
 export function parseWithDefault<D>(value: D | undefined, defaultValue: D): D {
   return value !== undefined ? value : defaultValue;
 }
-
-export const parseQuantizer = <T extends QuantizerConfig>(config?: T): T | undefined => {
-  if (config === undefined) {
-    return undefined;
-  }
-  if (QuantizerGuards.isPQCreate(config)) {
-    return {
-      ...config,
-      type: 'pq',
-    } as T;
-  } else if (QuantizerGuards.isBQCreate(config)) {
-    return {
-      ...config,
-      type: 'bq',
-    } as T;
-  }
-  return config;
-};
diff --git a/src/collections/configure/vectorIndex.ts b/src/collections/configure/vectorIndex.ts
@@ -16,8 +16,6 @@ import {
   VectorIndexConfigHNSWUpdate,
 } from './types/index.js';
 
-import { parseQuantizer } from './parsing.js';
-
 const isModuleConfig = <N, C>(config: ModuleConfig<N, C> | C): config is ModuleConfig<N, C> => {
   return config && typeof config === 'object' && 'name' in config && 'config' in config;
 };
@@ -40,7 +38,7 @@ const configure = {
       config: {
         distance,
         vectorCacheMaxObjects,
-        quantizer: parseQuantizer(quantizer),
+        quantizer: quantizer,
       },
     };
   },
@@ -62,7 +60,7 @@ const configure = {
         ? {
             ...rest,
             distance: distanceMetric,
-            quantizer: parseQuantizer(rest.quantizer),
+            quantizer: rest.quantizer,
           }
         : undefined,
     };
@@ -177,10 +175,7 @@ const reconfigure = {
   }): ModuleConfig<'flat', VectorIndexConfigFlatUpdate> => {
     return {
       name: 'flat',
-      config: {
-        vectorCacheMaxObjects: options.vectorCacheMaxObjects,
-        quantizer: parseQuantizer(options.quantizer),
-      },
+      config: options,
     };
   },
   /**
@@ -221,8 +216,8 @@ const reconfigure = {
      * NOTE: If the vector index already has a quantizer configured, you cannot change its quantizer type; only its values.
      * So if you want to change the quantizer type, you must recreate the collection.
      *
-     * @param {boolean} [options.cache] Whether to cache the quantizer. Default is false.
-     * @param {number} [options.rescoreLimit] The rescore limit. Default is 1000.
+     * @param {boolean} [options.cache] Whether to cache the quantizer.
+     * @param {number} [options.rescoreLimit] The new rescore limit.
      * @returns {BQConfigCreate} The configuration object.
      */
     bq: (options?: { cache?: boolean; rescoreLimit?: number }): BQConfigUpdate => {
@@ -237,11 +232,11 @@ const reconfigure = {
      * NOTE: If the vector index already has a quantizer configured, you cannot change its quantizer type; only its values.
      * So if you want to change the quantizer type, you must recreate the collection.
      *
-     * @param {number} [options.centroids] The number of centroids. Default is 256.
-     * @param {PQEncoderDistribution} [options.pqEncoderDistribution] The encoder distribution. Default is 'log-normal'.
-     * @param {PQEncoderType} [options.pqEncoderType] The encoder type. Default is 'kmeans'.
-     * @param {number} [options.segments] The number of segments. Default is 0.
-     * @param {number} [options.trainingLimit] The training limit. Default is 100000.
+     * @param {number} [options.centroids] The new number of centroids.
+     * @param {PQEncoderDistribution} [options.pqEncoderDistribution] The new encoder distribution.
+     * @param {PQEncoderType} [options.pqEncoderType] The new encoder type.
+     * @param {number} [options.segments] The new number of segments.
+     * @param {number} [options.trainingLimit] The new training limit.
      * @returns {PQConfigUpdate} The configuration object.
      */
     pq: (options?: {
@@ -270,8 +265,8 @@ const reconfigure = {
      * NOTE: If the vector index already has a quantizer configured, you cannot change its quantizer type; only its values.
      * So if you want to change the quantizer type, you must recreate the collection.
      *
-     * @param {number} [options.rescoreLimit] The rescore limit. Default is 1000.
-     * @param {number} [options.trainingLimit] The training limit. Default is 100000.
+     * @param {number} [options.rescoreLimit] The rescore limit.
+     * @param {number} [options.trainingLimit] The training limit.
      * @returns {SQConfigUpdate} The configuration object.
      */
     sq: (options?: { rescoreLimit?: number; trainingLimit?: number }): SQConfigUpdate => {