Copilot commented on code in PR #18091:
URL: https://github.com/apache/pinot/pull/18091#discussion_r3036192666
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/VectorIndexConfigValidator.java:
##########
@@ -121,6 +125,10 @@ private static void
validateBackendSpecificProperties(VectorIndexConfig config,
validateNoForeignProperties(properties, HNSW_PROPERTIES, "IVF_FLAT",
"HNSW");
validateIvfFlatProperties(properties);
break;
+ case IVF_PQ:
+ validateNoForeignProperties(properties, HNSW_PROPERTIES, "IVF_PQ",
"HNSW");
+ validateIvfPqProperties(config, properties);
+ break;
Review Comment:
`validateBackendSpecificProperties()` only rejects foreign keys for one
other backend (e.g., HNSW rejects IVF_FLAT keys, but not IVF_PQ keys). As a
result, backend-specific properties can still be mixed across
HNSW/IVF_FLAT/IVF_PQ (e.g., `pqM` on IVF_FLAT, or `minRowsForIndex` on IVF_PQ)
without validation errors, contradicting the stated validation rules. Consider
validating against *both* other backend key sets for each case (or using
allow-lists per backend and rejecting known keys from the other backends).
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/creator/impl/vector/pq/benchmark/VectorIndexBenchmark.java:
##########
@@ -0,0 +1,289 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package
org.apache.pinot.segment.local.segment.creator.impl.vector.pq.benchmark;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Random;
+import org.apache.commons.io.FileUtils;
+import
org.apache.pinot.segment.local.segment.creator.impl.vector.pq.IvfPqVectorIndexCreator;
+import
org.apache.pinot.segment.local.segment.creator.impl.vector.pq.VectorDistanceUtil;
+import
org.apache.pinot.segment.local.segment.index.readers.vector.IvfPqVectorIndexReader;
+import org.apache.pinot.segment.spi.index.creator.VectorIndexConfig;
+import
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+
+/**
+ * Reproducible benchmark suite for vector index backends.
+ *
+ * <p>Compares: exact scan, IVF_PQ (various configs).
+ * HNSW benchmark requires Lucene integration which is tested separately.
+ *
+ * <p>This benchmark is designed to run as a unit test but produces a formatted
+ * results table on stdout. It is NOT run as part of normal CI — invoke
explicitly with:
+ * {@code mvn -pl pinot-segment-local -Dtest=VectorIndexBenchmark test}
+ */
+public class VectorIndexBenchmark {
+ private static final int DIMENSION = 64;
+ private static final int NUM_VECTORS = 10_000;
+ private static final int NUM_QUERIES = 100;
+ private static final int[] TOP_K_VALUES = {10, 100};
+
+ @Test(enabled = false, description = "Manual benchmark — run with
-Dtest=VectorIndexBenchmark")
+ public void runBenchmark()
+ throws IOException {
+ System.out.println("=== Vector Index Benchmark ===");
+ System.out.println("Vectors: " + NUM_VECTORS + ", Dimension: " + DIMENSION
+ ", Queries: " + NUM_QUERIES);
+ System.out.println();
+
+ // Generate L2-oriented dataset (uniform random)
+ Random random = new Random(42);
+ float[][] l2Vectors = generateUniformVectors(NUM_VECTORS, DIMENSION,
random);
+ float[][] l2Queries = generateUniformVectors(NUM_QUERIES, DIMENSION, new
Random(999));
+
+ // Generate normalized cosine dataset
+ float[][] cosineVectors = generateNormalizedVectors(NUM_VECTORS,
DIMENSION, random);
+ float[][] cosineQueries = generateNormalizedVectors(NUM_QUERIES,
DIMENSION, new Random(999));
+
+ System.out.println("## L2/Euclidean Dataset");
+ benchmarkDataset("L2", l2Vectors, l2Queries, "EUCLIDEAN",
+ VectorIndexConfig.VectorDistanceFunction.EUCLIDEAN);
+
+ System.out.println();
+ System.out.println("## Cosine Dataset (normalized vectors)");
+ benchmarkDataset("Cosine", cosineVectors, cosineQueries, "COSINE",
+ VectorIndexConfig.VectorDistanceFunction.COSINE);
+ }
+
+ private void benchmarkDataset(String label, float[][] vectors, float[][]
queries, String distFuncStr,
+ VectorIndexConfig.VectorDistanceFunction distFunc)
+ throws IOException {
+ int distFuncCode = distFunc ==
VectorIndexConfig.VectorDistanceFunction.EUCLIDEAN ? 1 : 0;
+
+ // Precompute exact results for recall computation
+ Map<Integer, int[][]> exactResults = new HashMap<>();
+ for (int topK : TOP_K_VALUES) {
+ int[][] exact = new int[queries.length][];
+ for (int q = 0; q < queries.length; q++) {
+ exact[q] = findExactTopK(queries[q], vectors, topK, distFuncCode);
+ }
+ exactResults.put(topK, exact);
+ }
+
+ // Exact scan benchmark
+ benchmarkExactScan(label, vectors, queries, exactResults, distFuncCode);
+
+ // IVF_PQ configs to benchmark
+ int[][] configs = {
+ // {nlist, pqM, pqNbits, nprobe}
+ {64, 8, 8, 4},
+ {64, 8, 8, 8},
+ {64, 8, 8, 16},
+ {128, 16, 8, 8},
+ {128, 16, 8, 16},
+ {256, 16, 8, 16},
+ {256, 16, 8, 32},
+ };
+
+ printHeader();
+ for (int[] cfg : configs) {
+ benchmarkIvfPq(label, vectors, queries, exactResults, distFuncStr,
distFunc, cfg[0], cfg[1], cfg[2], cfg[3]);
+ }
+
+ // Rerank on/off comparison
+ System.out.println();
+ System.out.println("### Rerank Effect (nlist=128, pqM=16, pqNbits=8,
nprobe=16)");
+ printHeader();
+ benchmarkIvfPqRerank(label, vectors, queries, exactResults, distFuncStr,
distFunc, 128, 16, 8, 16, true);
+ benchmarkIvfPqRerank(label, vectors, queries, exactResults, distFuncStr,
distFunc, 128, 16, 8, 16, false);
+ }
+
+ private void benchmarkExactScan(String label, float[][] vectors, float[][]
queries,
+ Map<Integer, int[][]> exactResults, int distFuncCode) {
+ System.out.println("### Exact Scan");
+ System.out.printf("| %-35s | %10s | %10s | %10s | %10s | %10s | %10s |%n",
+ "Config", "BuildMs", "SizeMB", "Recall@10", "Recall@100", "p50us",
"p95us");
+ System.out.println("|" + "-".repeat(37) + "|" + ("-".repeat(12) +
"|").repeat(6));
+
+ long[] latencies = new long[queries.length];
+ for (int q = 0; q < queries.length; q++) {
+ long start = System.nanoTime();
+ findExactTopK(queries[q], vectors, 100, distFuncCode);
+ latencies[q] = (System.nanoTime() - start) / 1000; // microseconds
+ }
+ Arrays.sort(latencies);
+ System.out.printf("| %-35s | %10d | %10.2f | %10.4f | %10.4f | %10d | %10d
|%n",
+ "ExactScan", 0, 0.0, 1.0, 1.0,
+ latencies[(int) (queries.length * 0.5)], latencies[(int)
(queries.length * 0.95)]);
+ System.out.println();
+ }
+
+ private void printHeader() {
+ System.out.printf("| %-35s | %10s | %10s | %10s | %10s | %10s | %10s |%n",
+ "Config", "BuildMs", "SizeMB", "Recall@10", "Recall@100", "p50us",
"p95us");
+ System.out.println("|" + "-".repeat(37) + "|" + ("-".repeat(12) +
"|").repeat(6));
+ }
+
+ private void benchmarkIvfPq(String label, float[][] vectors, float[][]
queries,
+ Map<Integer, int[][]> exactResults, String distFuncStr,
VectorIndexConfig.VectorDistanceFunction distFunc,
+ int nlist, int pqM, int pqNbits, int nprobe)
+ throws IOException {
+ benchmarkIvfPqRerank(label, vectors, queries, exactResults, distFuncStr,
distFunc,
+ nlist, pqM, pqNbits, nprobe, true);
+ }
+
+ private void benchmarkIvfPqRerank(String label, float[][] vectors, float[][]
queries,
+ Map<Integer, int[][]> exactResults, String distFuncStr,
VectorIndexConfig.VectorDistanceFunction distFunc,
+ int nlist, int pqM, int pqNbits, int nprobe, boolean rerank)
+ throws IOException {
+ File tempDir = new File(FileUtils.getTempDirectory(), "bench_" +
System.nanoTime());
+ tempDir.mkdirs();
+
+ try {
+ VectorIndexConfig config = createConfig(distFuncStr, distFunc, nlist,
pqM, pqNbits);
+
+ // Build
+ long buildStart = System.nanoTime();
+ try (IvfPqVectorIndexCreator creator = new
IvfPqVectorIndexCreator("emb", tempDir, config)) {
+ for (float[] v : vectors) {
+ creator.add(v);
+ }
+ creator.seal();
+ }
+ long buildMs = (System.nanoTime() - buildStart) / 1_000_000;
+
+ File indexFile = new File(tempDir, "emb" +
IvfPqVectorIndexCreator.FILE_EXTENSION);
+ double sizeMB = indexFile.length() / (1024.0 * 1024.0);
+
+ // Search
+ Map<String, String> opts = new HashMap<>();
+ opts.put(QueryOptionKey.VECTOR_NPROBE, String.valueOf(nprobe));
+ opts.put(QueryOptionKey.VECTOR_EXACT_RERANK, String.valueOf(rerank));
+
+ long[] latencies = new long[queries.length];
+ double recall10 = 0;
+ double recall100 = 0;
+
+ try (IvfPqVectorIndexReader reader = new IvfPqVectorIndexReader("emb",
tempDir, vectors.length, nprobe)) {
+ for (int q = 0; q < queries.length; q++) {
+ long start = System.nanoTime();
+ ImmutableRoaringBitmap result10 = reader.getDocIds(queries[q], 10,
opts);
+ latencies[q] = (System.nanoTime() - start) / 1000;
+ recall10 += computeRecall(result10, exactResults.get(10)[q]);
+
+ ImmutableRoaringBitmap result100 = reader.getDocIds(queries[q], 100,
opts);
+ recall100 += computeRecall(result100, exactResults.get(100)[q]);
Review Comment:
This benchmark passes query options via `reader.getDocIds(query, topK,
opts)` (nprobe + rerank), but `VectorIndexReader#getDocIds(..., searchParams)`
currently defaults to ignoring `searchParams`, and `IvfPqVectorIndexReader`
does not override it. As written, `vectorNprobe` / `vectorExactRerank` in
`opts` have no effect, so the reported benchmark numbers are misleading.
Consider either (a) overriding `getDocIds(..., searchParams)` in IVF_PQ to
honor `vectorNprobe` (without mutating shared state), and removing
`vectorExactRerank` from the reader-level benchmark (since rerank happens at
operator layer), or (b) explicitly calling `setNprobe()` and implementing
rerank logic in the benchmark itself.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/VectorSimilarityFilterOperator.java:
##########
@@ -136,12 +137,27 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- return EXPLAIN_NAME + "(indexLookUp:vector_index"
- + ", operator:" + _predicate.getType()
- + ", vector identifier:" + _predicate.getLhs().getIdentifier()
- + ", vector literal:" + Arrays.toString(_predicate.getValue())
- + ", topK to search:" + _predicate.getTopK()
- + ')';
+ Map<String, String> debugInfo = _vectorIndexReader.getIndexDebugInfo();
+ StringBuilder sb = new StringBuilder();
+ sb.append(EXPLAIN_NAME);
+ sb.append("(indexLookUp:vector_index");
+ sb.append(", operator:").append(_predicate.getType());
+ sb.append(", vector
identifier:").append(_predicate.getLhs().getIdentifier());
+ sb.append(", vector
literal:").append(Arrays.toString(_predicate.getValue()));
+ sb.append(", topK to search:").append(_predicate.getTopK());
+ if (!debugInfo.isEmpty()) {
+ sb.append(", backend:").append(debugInfo.getOrDefault("backend",
"UNKNOWN"));
+ String nprobe = debugInfo.get("nprobe");
+ if (nprobe != null) {
+ sb.append(", nprobe:").append(nprobe);
+ }
+ String rerank = debugInfo.get("exactRerank");
+ if (rerank != null) {
+ sb.append(", exactRerank:").append(rerank);
+ }
+ }
+ sb.append(')');
+ return sb.toString();
Review Comment:
`toExplainString()` only appends backend/nprobe details when
`getIndexDebugInfo()` is non-empty. Currently, IVF_FLAT readers don’t implement
`getIndexDebugInfo()`, so EXPLAIN omits backend details for IVF_FLAT queries
even though the PR description suggests backend should be shown. Consider
either adding `getIndexDebugInfo()` to `IvfFlatVectorIndexReader` (e.g.,
backend=IVF_FLAT, nprobe=...) or enhancing this method to fall back to
`instanceof NprobeAware` to still populate backend info.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/VectorIndexUtils.java:
##########
@@ -56,12 +58,52 @@ static void cleanupVectorIndex(File segDir, String column) {
// Remove the IVF_FLAT index file
File ivfFlatIndexFile = new File(segDir, column +
Indexes.VECTOR_IVF_FLAT_INDEX_FILE_EXTENSION);
FileUtils.deleteQuietly(ivfFlatIndexFile);
+
+ // Remove IVF_PQ index file
+ File ivfPqIndexFile = new File(segDir, column +
Indexes.VECTOR_IVFPQ_INDEX_FILE_EXTENSION);
+ FileUtils.deleteQuietly(ivfPqIndexFile);
}
static boolean hasVectorIndex(File segDir, String column) {
return new File(segDir, column +
Indexes.VECTOR_V912_HNSW_INDEX_FILE_EXTENSION).exists()
|| new File(segDir, column +
Indexes.VECTOR_V912_INDEX_FILE_EXTENSION).exists()
- || new File(segDir, column +
Indexes.VECTOR_IVF_FLAT_INDEX_FILE_EXTENSION).exists();
+ || new File(segDir, column +
Indexes.VECTOR_IVF_FLAT_INDEX_FILE_EXTENSION).exists()
+ || new File(segDir, column +
Indexes.VECTOR_IVFPQ_INDEX_FILE_EXTENSION).exists();
Review Comment:
`cleanupVectorIndex()` and `hasVectorIndex()` only check/delete files in the
segment root directory. For V3 segments, vector index files can live under the
V3 subdirectory (as handled by the new `detectVectorIndexBackend()`), so these
methods can (a) fail to detect existing vector indexes and (b) leave stale
vector index files behind when removing/rebuilding indexes. Consider mirroring
the V3-aware logic here (check/delete in both `segDir` and
`SegmentDirectoryPaths.findSegmentDirectory(segDir)` when different).
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/creator/VectorIndexConfigValidator.java:
##########
@@ -49,6 +49,10 @@ public final class VectorIndexConfigValidator {
static final Set<String> IVF_FLAT_PROPERTIES =
Collections.unmodifiableSet(new HashSet<>(
Arrays.asList("nlist", "trainSampleSize", "trainingSeed",
"minRowsForIndex")));
+ // IVF_PQ-specific property keys
+ static final Set<String> IVF_PQ_PROPERTIES = Collections.unmodifiableSet(new
HashSet<>(
+ Arrays.asList("nlist", "pqM", "pqNbits", "nprobe", "trainSampleSize",
"trainingSeed")));
+
Review Comment:
`IVF_PQ_PROPERTIES` is declared but never referenced in this class (only
`HNSW_PROPERTIES` and `IVF_FLAT_PROPERTIES` are used in
`validateNoForeignProperties`). Either wire this set into the foreign-property
validation logic or remove it to avoid dead code and configuration drift.
```suggestion
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/vector/VectorIndexType.java:
##########
@@ -200,6 +216,11 @@ public MutableIndex createMutableIndex(MutableIndexContext
context, VectorIndexC
.isSingleValueField()) {
return null;
}
+ // IVF_PQ does not support mutable/realtime segments. Return null so the
+ // consuming segment falls back to exact scan rather than silently using
HNSW.
+ if ("IVF_PQ".equals(config.getVectorIndexType())) {
+ return null;
+ }
Review Comment:
`createMutableIndex()` returns early when `config.getVectorIndexType()` is
exactly `"IVF_PQ"`, which bypasses the `switch` and its WARN log for
unsupported mutable IVF_PQ. This makes the IVF_PQ realtime fallback silent and
also leaves a now-unreachable `case IVF_PQ` block below. Consider removing the
early return and letting the `IVF_PQ` switch case handle the (logged) fallback
consistently.
```suggestion
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/vector/IvfPqVectorIndexReader.java:
##########
@@ -0,0 +1,292 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.segment.index.readers.vector;
+
+import java.io.Closeable;
+import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.ByteOrder;
+import java.nio.MappedByteBuffer;
+import java.nio.channels.FileChannel;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.PriorityQueue;
+import
org.apache.pinot.segment.local.segment.creator.impl.vector.pq.IvfPqIndexFormat;
+import
org.apache.pinot.segment.local.segment.creator.impl.vector.pq.IvfPqVectorIndexCreator;
+import org.apache.pinot.segment.local.segment.creator.impl.vector.pq.KMeans;
+import
org.apache.pinot.segment.local.segment.creator.impl.vector.pq.ProductQuantizer;
+import
org.apache.pinot.segment.local.segment.creator.impl.vector.pq.VectorDistanceUtil;
+import org.apache.pinot.segment.spi.index.reader.NprobeAware;
+import org.apache.pinot.segment.spi.index.reader.VectorIndexReader;
+import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Reader for IVF_PQ vector index. Loads the compact index (no original
vectors)
+ * and performs approximate nearest-neighbor search using IVF + Product
Quantization.
+ *
+ * <p>Exact rerank is handled by the operator layer via the forward index
reader,
+ * not by this reader. This keeps the index file compact (only PQ codes +
docIds).</p>
+ *
+ * <p>Search flow:
+ * <ol>
+ * <li>Find the nprobe closest coarse centroids to the query vector</li>
+ * <li>Score all candidates in probed lists using approximate PQ distances
(ADC)</li>
+ * <li>Return top-K approximate candidates as doc IDs</li>
+ * </ol>
+ */
+public class IvfPqVectorIndexReader implements VectorIndexReader, NprobeAware {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(IvfPqVectorIndexReader.class);
+ private static final int DEFAULT_NPROBE = 8;
+
+ private final String _column;
+ private final int _dimension;
+ private final int _nlist;
+ private final int _pqM;
+ private final int _pqNbits;
+ private final int _numVectors;
+ private final int _distanceFunctionCode;
+
+ private final float[][] _coarseCentroids;
+ private final ProductQuantizer _pq;
+
+ // Inverted lists: docIds and PQ codes per list
+ private final int[][] _listDocIds;
+ private final byte[][][] _listPqCodes;
+
+ private volatile int _nprobe;
+ private final Closeable _closeHandle;
+
+ public IvfPqVectorIndexReader(String column, File segmentDir, int numDocs,
int nprobe) {
+ _column = column;
+ _nprobe = nprobe > 0 ? nprobe : DEFAULT_NPROBE;
+
+ File indexFile = findIndexFile(segmentDir);
+ if (indexFile == null) {
+ throw new IllegalStateException("Failed to find IVF_PQ index file for
column: " + column);
+ }
+
+ try {
+ RandomAccessFile raf = new RandomAccessFile(indexFile, "r");
+ MappedByteBuffer buffer =
raf.getChannel().map(FileChannel.MapMode.READ_ONLY, 0, raf.length());
+ buffer.order(ByteOrder.LITTLE_ENDIAN);
+ _closeHandle = raf;
+
+ // Read header
+ int magic = buffer.getInt();
+ if (magic != IvfPqIndexFormat.MAGIC) {
+ throw new IOException("Invalid IVF_PQ index magic: " +
Integer.toHexString(magic));
+ }
+ int version = buffer.getInt();
+ if (version != IvfPqIndexFormat.VERSION) {
+ throw new IOException("Unsupported IVF_PQ index version: " + version);
+ }
+ _dimension = buffer.getInt();
+ _nlist = buffer.getInt();
+ _pqM = buffer.getInt();
+ _pqNbits = buffer.getInt();
+ _distanceFunctionCode = buffer.getInt();
+ _numVectors = buffer.getInt();
+
+ int ksub = 1 << _pqNbits;
+ int dsub = _pqM > 0 ? _dimension / _pqM : 1;
+
+ if (_numVectors == 0 || _nlist == 0) {
+ _coarseCentroids = new float[0][];
+ int safeDim = Math.max(_dimension, 1);
+ int safeM = Math.max(_pqM, 1);
+ if (safeDim % safeM != 0) {
+ safeM = 1;
+ }
+ _pq = new ProductQuantizer(safeDim, safeM, Math.max(_pqNbits, 1));
+ _listDocIds = new int[0][];
+ _listPqCodes = new byte[0][][];
+ return;
+ }
+
+ // Read coarse centroids
+ _coarseCentroids = new float[_nlist][_dimension];
+ for (int c = 0; c < _nlist; c++) {
+ for (int d = 0; d < _dimension; d++) {
+ _coarseCentroids[c][d] = buffer.getFloat();
+ }
+ }
+
+ // Read PQ codebooks
+ float[][][] codebooks = new float[_pqM][ksub][dsub];
+ for (int sub = 0; sub < _pqM; sub++) {
+ for (int code = 0; code < ksub; code++) {
+ for (int d = 0; d < dsub; d++) {
+ codebooks[sub][code][d] = buffer.getFloat();
+ }
+ }
+ }
+ _pq = new ProductQuantizer(_dimension, _pqM, _pqNbits);
+ _pq.setCodebooks(codebooks);
+
+ // Read inverted lists (docIds + PQ codes only — no original vectors)
+ _listDocIds = new int[_nlist][];
+ _listPqCodes = new byte[_nlist][][];
+ for (int list = 0; list < _nlist; list++) {
+ int listSize = buffer.getInt();
+ _listDocIds[list] = new int[listSize];
+ for (int i = 0; i < listSize; i++) {
+ _listDocIds[list][i] = buffer.getInt();
+ }
+ _listPqCodes[list] = new byte[listSize][_pqM];
+ for (int i = 0; i < listSize; i++) {
+ buffer.get(_listPqCodes[list][i]);
+ }
+ }
+
+ LOGGER.info("Loaded IVF_PQ index for column: {} with {} lists, {}
vectors, dimension={}, pqM={}, pqNbits={}",
+ column, _nlist, _numVectors, _dimension, _pqM, _pqNbits);
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to load IVF_PQ index for column: " +
column, e);
+ }
+ }
+
+ @Override
+ public MutableRoaringBitmap getDocIds(float[] queryVector, int topK) {
+ if (_numVectors == 0 || _nlist == 0) {
+ return new MutableRoaringBitmap();
+ }
+
+ // For COSINE and INNER_PRODUCT, normalize query to match the normalized
index vectors.
+ // L2 on unit-normalized vectors preserves both cosine and inner-product
ranking.
+ float[] searchVector = (_distanceFunctionCode ==
IvfPqIndexFormat.DIST_COSINE
+ || _distanceFunctionCode == IvfPqIndexFormat.DIST_INNER_PRODUCT)
Review Comment:
The INNER_PRODUCT path normalizes vectors and then uses L2-based centroid
probing + PQ ADC distances (see comment "L2 on unit-normalized vectors
preserves ... inner-product ranking"). This changes the semantics from maximum
inner product to cosine-on-normalized-vectors and does **not** match the
existing IVF_FLAT behavior (which uses `-dotProduct` for
INNER_PRODUCT/DOT_PRODUCT) or Lucene's `MAXIMUM_INNER_PRODUCT`. Either
disable/forbid INNER_PRODUCT for IVF_PQ (fail validation) or implement a
distance computation that preserves true inner-product semantics for this
backend.
```suggestion
if (_distanceFunctionCode == IvfPqIndexFormat.DIST_INNER_PRODUCT) {
throw new UnsupportedOperationException(
"INNER_PRODUCT is not supported for IVF_PQ because the current
implementation uses "
+ "normalized vectors with L2-based centroid probing and PQ
ADC scoring, which does not preserve "
+ "true maximum inner-product semantics");
}
if (_numVectors == 0 || _nlist == 0) {
return new MutableRoaringBitmap();
}
// For COSINE, normalize query to match the normalized index vectors.
float[] searchVector = _distanceFunctionCode ==
IvfPqIndexFormat.DIST_COSINE
```
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/VectorSimilarityFilterOperator.java:
##########
@@ -160,6 +176,14 @@ protected void explainAttributes(ExplainAttributeBuilder
attributeBuilder) {
if (_searchParams.isExactRerank()) {
attributeBuilder.putString("exactRerank", "true");
}
+ Map<String, String> debugInfo = _vectorIndexReader.getIndexDebugInfo();
+ if (!debugInfo.isEmpty()) {
+ attributeBuilder.putString("backend", debugInfo.getOrDefault("backend",
"UNKNOWN"));
+ String nprobe = debugInfo.get("nprobe");
+ if (nprobe != null) {
+ attributeBuilder.putString("nprobe", nprobe);
+ }
+ }
Review Comment:
PR description indicates operator-level exact rerank should use the
segment’s configured distance function, and EXPLAIN output should surface
effective rerank/nprobe settings. However, `applyExactRerank()` in this class
still hardcodes L2 distance (see existing TODO in that method), so rerank can
produce incorrect ordering for COSINE / INNER_PRODUCT segments (including
IVF_PQ). Consider plumbing the configured distance function into rerank and
reflecting effective settings in explain output (use `_searchParams` for
rerank/nprobe instead of relying on reader debugInfo keys like `exactRerank`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
