This is probably not going to do what we want when a PerFieldKnnVectorsFormat is used, as this would check the limit on PerFieldKnnVectorsFormat, rather than on the actual format that is used for pf.fieldName. Maybe getMaxDimensions should be on KnnVectorsWriter and we could forward to pf.knnVectorsWrtier here for checking?

Actually my suggestion wouldn't work, as the writer would already be created with the number of dimensions of the field type when we run the check. So I guess we either need to add the field name to getMaxDimensions or make the codec responsible for performing the check rather than IndexingChain.

@jpountz Thanks for your feedback, you are completely right, I missed about PerFieldKnnVectorsFormat.

In 21ebd51, I've added a field name to the getMaxDimensions method.

I worry that this adds a hashtable lookup on a hot code path. Maybe it's not that bad for vectors, which are slow to index anyway, but I'd rather avoid it. What about making the codec responsible for checking the limit? Something like below:

diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsFormat.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsFormat.java
index cb3e5ef8b10..6c365e53528 100644
--- a/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsFormat.java
+++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsFormat.java
@@ -108,6 +108,9 @@ public final class Lucene95HnswVectorsFormat extends KnnVectorsFormat {
   public static final int VERSION_START = 0;
   public static final int VERSION_CURRENT = VERSION_START;
 
+  /** A maximum number of vector dimensions supported by this codeс */
+  public static final int MAX_DIMENSIONS = 1024;
+
   /**
    * A maximum configurable maximum max conn.
    *
@@ -177,7 +180,7 @@ public final class Lucene95HnswVectorsFormat extends KnnVectorsFormat {
 
   @Override
   public KnnVectorsWriter fieldsWriter(SegmentWriteState state) throws IOException {
-    return new Lucene95HnswVectorsWriter(state, maxConn, beamWidth);
+    return new Lucene95HnswVectorsWriter(state, maxConn, beamWidth, MAX_DIMENSIONS);
   }
 
   @Override
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsWriter.java
index 5358d66f16e..196f12a21ad 100644
--- a/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsWriter.java
+++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsWriter.java
@@ -60,13 +60,15 @@ public final class Lucene95HnswVectorsWriter extends KnnVectorsWriter {
   private final IndexOutput meta, vectorData, vectorIndex;
   private final int M;
   private final int beamWidth;
+  private final int maxDimension;
 
   private final List<FieldWriter<?>> fields = new ArrayList<>();
   private boolean finished;
 
-  Lucene95HnswVectorsWriter(SegmentWriteState state, int M, int beamWidth) throws IOException {
+  Lucene95HnswVectorsWriter(SegmentWriteState state, int M, int beamWidth, int maxDimension) throws IOException {
     this.M = M;
     this.beamWidth = beamWidth;
+    this.maxDimension = maxDimension;
     segmentWriteState = state;
     String metaFileName =
         IndexFileNames.segmentFileName(
@@ -117,6 +119,9 @@ public final class Lucene95HnswVectorsWriter extends KnnVectorsWriter {
 
   @Override
   public KnnFieldVectorsWriter<?> addField(FieldInfo fieldInfo) throws IOException {
+    if (fieldInfo.getVectorDimension() > maxDimension) {
+      throw new IllegalArgumentException("Number of dimensions " + fieldInfo.getVectorDimension() + " for field " + fieldInfo.name + " exceeds the limit of " + maxDimension);
+    }
     FieldWriter<?> newField =
         FieldWriter.create(fieldInfo, M, beamWidth, segmentWriteState.infoStream);
     fields.add(newField);

@jpountz Thank you for the additional feedback.

I worry that this adds a hashtable lookup on a hot code path. Maybe it's not that bad for vectors, which are slow to index anyway, but I'd rather avoid it.

This is not really a hot code path. We ask for getCodec().knnVectorsFormat().getMaxDimensions in the initializeFieldInfo function, that happens only once per a new field per segment.

What about making the codec responsible for checking the limit?

Thanks for the suggestion, I experimented with this idea, and encountered the following difficulty with it:

• we need to create a new FieldInfo before passing it to KnnFieldVectorsWriter<?> addField(FieldInfo fieldInfo).
• The way we create it is : FieldInfo fi = fieldInfos.add( by adding to the global fieldInfos. This means that if FieldInfo contains incorrect number of dimensions, it will be stored like this in the global fieldInfos, and we can't change it (for example with a second document with correct number of dims).

May be as an alternative we can do a validation as a separate method of KnnVectorsWriter:

public void validateFieldDims(int dims)

What do you think?

Ohhh thanks for explaining, I had not fully understood how your change worked. I like that we're retaining the property that the field info doesn't even get created if its number of dimensions is above the limit.

Yes this looks fine. The hashtable lookup is no issue at all.

As getMaxDimensions() is a public codec method, we can let us do the check here. A separate validateFieldDims() is not needed.

I only don't like the long call chain to actually get the vectors format from the indexWriterConfig. But I can live with that.

In main branch this should be abstract, only in 9.x we should have a default impl.

is this the only subclass of KnnVectorsFormat that we have?

In addition, we should also add explicit number of dimensions in backwards codecs, because at the time when it was implemented 1024 was their default. In backwards codec the method should be final, IMHO.

We have SimpleTextKnnVectorsFormat too.

So basically the check is now delayed to addDocument()? Great!

I was afraid that the check comes delayed while indexing of flushing is happening. So to me this looks good.

Yes this looks fine. The hashtable lookup is no issue at all.

As getMaxDimensions() is a public codec method, we can let us do the check here. A separate validateFieldDims() is not needed.

I only don't like the long call chain to actually get the vectors format from the indexWriterConfig. But I can live with that.

minor: #12605 proposes to have a space before the vector's dimensions words here.