Let production code read a label's stored vectors

src/VecSim/algorithms/brute_force/brute_force_multi.h

@@ -40,21 +40,36 @@ class BruteForceIndex_Multi : public BruteForceIndex<DataType, DistType> {
     }
     std::unordered_map<idType, std::pair<idType, labelType>>
     deleteVectorAndGetUpdatedIds(labelType label) override;
-#ifdef BUILD_TESTS
     void getDataByLabel(labelType label,
                         std::vector<std::vector<DataType>> &vectors_output) const override {
-
+        // A quantized element is not the elements: SQ8 keeps one byte per dimension plus FP32
+        // metadata, so copying `dim * sizeof(DataType)` out of it would reinterpret compression
+        // and metadata as values. Nothing here dequantizes, so the honest answer is none -- per
+        // the contract an empty output reads as "cannot tell".
+        //
+        // Asking the index rather than comparing sizes: the quantized element is only *smaller*
+        // than the raw elements above a certain dimension. At dim 4 with FP32/L2 it is larger
+        // (4 + 4*4 = 20 bytes against 16), so a size test concludes "not quantized" exactly where
+        // it matters most.
+        if (this->isQuantized) {
+            return;
+        }
         auto ids = labelToIdsLookup.find(label);
+        if (ids == labelToIdsLookup.end()) {
+            return;
+        }
 
         for (idType id : ids->second) {
             auto vec = std::vector<DataType>(this->dim);
             // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like
             // the norm
             memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType));
-            vectors_output.push_back(vec);
+            vectors_output.push_back(std::move(vec));
         }
     }
 
+#ifdef BUILD_TESTS
+
     std::vector<std::vector<char>> getStoredVectorDataByLabel(labelType label) const override {
         std::vector<std::vector<char>> vectors_output;
         auto ids = labelToIdsLookup.find(label);

src/VecSim/algorithms/brute_force/brute_force_single.h

@@ -43,19 +43,34 @@ class BruteForceIndex_Single : public BruteForceIndex<DataType, DistType> {
     // We call this when we KNOW that the label exists in the index.
     idType getIdOfLabel(labelType label) const { return labelToIdLookup.find(label)->second; }
 
-#ifdef BUILD_TESTS
     void getDataByLabel(labelType label,
                         std::vector<std::vector<DataType>> &vectors_output) const override {
-
-        auto id = labelToIdLookup.at(label);
+        // A quantized element is not the elements: SQ8 keeps one byte per dimension plus FP32
+        // metadata, so copying `dim * sizeof(DataType)` out of it would reinterpret compression
+        // and metadata as values. Nothing here dequantizes, so the honest answer is none -- per
+        // the contract an empty output reads as "cannot tell".
+        //
+        // Asking the index rather than comparing sizes: the quantized element is only *smaller*
+        // than the raw elements above a certain dimension. At dim 4 with FP32/L2 it is larger
+        // (4 + 4*4 = 20 bytes against 16), so a size test concludes "not quantized" exactly where
+        // it matters most.
+        if (this->isQuantized) {
+            return;
+        }
+        auto it = labelToIdLookup.find(label);
+        if (it == labelToIdLookup.end()) {
+            return;
+        }
 
         auto vec = std::vector<DataType>(this->dim);
         // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like the
         // norm
-        memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType));
-        vectors_output.push_back(vec);
+        memcpy(vec.data(), this->getDataByInternalId(it->second), this->dim * sizeof(DataType));
+        vectors_output.push_back(std::move(vec));
     }
 
+#ifdef BUILD_TESTS
+
     std::vector<std::vector<char>> getStoredVectorDataByLabel(labelType label) const override {
         std::vector<std::vector<char>> vectors_output;
         auto id = labelToIdLookup.at(label);

src/VecSim/algorithms/hnsw/hnsw_multi.h

@@ -68,20 +68,40 @@ class HNSWIndex_Multi : public HNSWIndex<DataType, DistType> {
         : HNSWIndex<DataType, DistType>(input, params, abstractInitParams, components, version),
           labelLookup(this->maxElements, this->allocator) {}
 
+#endif
     void getDataByLabel(labelType label,
                         std::vector<std::vector<DataType>> &vectors_output) const override {
-
+        // See the single-value implementation: the guard is the accessor's to take, because
+        // `indexDataGuard` -- not the main lock -- is what ingest and `markDelete` hold while
+        // mutating `labelLookup` and the data blocks.
+        // A quantized element is not the elements: SQ8 keeps one byte per dimension plus FP32
+        // metadata, so copying `dim * sizeof(DataType)` out of it would reinterpret compression
+        // and metadata as values. Nothing here dequantizes, so the honest answer is none -- per
+        // the contract an empty output reads as "cannot tell".
+        //
+        // Asking the index rather than comparing sizes: the quantized element is only *smaller*
+        // than the raw elements above a certain dimension. At dim 4 with FP32/L2 it is larger
+        // (4 + 4*4 = 20 bytes against 16), so a size test concludes "not quantized" exactly where
+        // it matters most.
+        if (this->isQuantized) {
+            return;
+        }
+        std::shared_lock<std::shared_mutex> index_data_lock(this->indexDataGuard);
         auto ids = labelLookup.find(label);
+        if (ids == labelLookup.end()) {
+            return;
+        }
 
         for (idType id : ids->second) {
             auto vec = std::vector<DataType>(this->dim);
             // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like
             // the norm
             memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType));
-            vectors_output.push_back(vec);
+            vectors_output.push_back(std::move(vec));
         }
     }
 
+#ifdef BUILD_TESTS
     std::vector<std::vector<char>> getStoredVectorDataByLabel(labelType label) const override {
         std::vector<std::vector<char>> vectors_output;
         auto ids = labelLookup.find(label);

src/VecSim/algorithms/hnsw/hnsw_single.h

@@ -44,18 +44,39 @@ class HNSWIndex_Single : public HNSWIndex<DataType, DistType> {
         : HNSWIndex<DataType, DistType>(input, params, abstractInitParams, components, version),
           labelLookup(this->maxElements, this->allocator) {}
 
+#endif
     void getDataByLabel(labelType label,
                         std::vector<std::vector<DataType>> &vectors_output) const override {
-
-        auto id = labelLookup.at(label);
+        // `labelLookup` and the data blocks are mutated under `indexDataGuard` by callers that
+        // hold no more than a shared main lock -- tiered ingest and `markDelete` both do -- so
+        // reading them needs this guard, not the caller's. Same reason `getLabelsSet` takes it.
+        // A quantized element is not the elements: SQ8 keeps one byte per dimension plus FP32
+        // metadata, so copying `dim * sizeof(DataType)` out of it would reinterpret compression
+        // and metadata as values. Nothing here dequantizes, so the honest answer is none -- per
+        // the contract an empty output reads as "cannot tell".
+        //
+        // Asking the index rather than comparing sizes: the quantized element is only *smaller*
+        // than the raw elements above a certain dimension. At dim 4 with FP32/L2 it is larger
+        // (4 + 4*4 = 20 bytes against 16), so a size test concludes "not quantized" exactly where
+        // it matters most.
+        if (this->isQuantized) {
+            return;
+        }
+        std::shared_lock<std::shared_mutex> index_data_lock(this->indexDataGuard);
+        auto it = labelLookup.find(label);
+        if (it == labelLookup.end()) {
+            return;
+        }
 
         auto vec = std::vector<DataType>(this->dim);
         // Only copy the vector data (dim * sizeof(DataType)), not any additional metadata like the
         // norm
-        memcpy(vec.data(), this->getDataByInternalId(id), this->dim * sizeof(DataType));
-        vectors_output.push_back(vec);
+        memcpy(vec.data(), this->getDataByInternalId(it->second), this->dim * sizeof(DataType));
+        vectors_output.push_back(std::move(vec));
     }
 
+#ifdef BUILD_TESTS
+
     std::vector<std::vector<char>> getStoredVectorDataByLabel(labelType label) const override {
         std::vector<std::vector<char>> vectors_output;
         auto id = labelLookup.at(label);

src/VecSim/algorithms/hnsw/hnsw_tiered.h

@@ -261,7 +261,6 @@ class TieredHNSWIndex : public VecSimTieredIndex<DataType, DistType> {
     }
 
 #ifdef BUILD_TESTS
-    void getDataByLabel(labelType label, std::vector<std::vector<DataType>> &vectors_output) const;
     size_t indexMetaDataCapacity() const override {
         return this->backendIndex->indexMetaDataCapacity() +
                this->frontendIndex->indexMetaDataCapacity();
@@ -1343,12 +1342,3 @@ VecSimIndexBasicInfo TieredHNSWIndex<DataType, DistType>::basicInfo() const {
     info.algo = VecSimAlgo_HNSWLIB;
     return info;
 }
-
-#ifdef BUILD_TESTS
-template <typename DataType, typename DistType>
-void TieredHNSWIndex<DataType, DistType>::getDataByLabel(
-    labelType label, std::vector<std::vector<DataType>> &vectors_output) const {
-    this->getHNSWIndex()->getDataByLabel(label, vectors_output);
-}
-
-#endif

src/VecSim/algorithms/hnsw/hnsw_tiered_tests_friends.h

@@ -42,6 +42,7 @@ INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTest_RangeSearch_Test)
 INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTest_parallelRangeSearch_Test)
 
 INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_insertJobAsync_Test)
+INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_getDataByLabelSpansBothTiers_Test)
 INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_insertJobAsyncMulti_Test)
 INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_KNNSearch_Test)
 INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_MergeMulti_Test)

src/VecSim/algorithms/svs/svs.h

@@ -799,14 +799,37 @@ class SVSIndex : public VecSimIndexAbstract<svs_details::vecsim_dt<DataType>, fl
             return vectors_output;
         }
     }
+    svs::logging::logger_ptr getLogger() const override { return logger_; }
+#endif
+
+    // TODO(MOD-17706): implement, and remove the SVSIndexBase check in
+    // VecSimTieredIndex::getDataByLabel that currently skips the backend read for SVS entirely.
+    //
+    // What it has to produce: the vectors stored under `label`, in the form the base contract
+    // describes -- the *stored* elements, i.e. after whatever preprocessing an insert applied --
+    // appending nothing when the label is absent, so the output size answers "is it held".
+    //
+    // Why it is empty today: SVS keeps vectors in the SVS library's own layout, quantized and for
+    // LeanVec dimensionality-reduced, and this wrapper has no per-label read of them.
+    //
+    // One rule to carry over from the HNSW implementations: report nothing rather than an
+    // approximation. They refuse when `isQuantized`, because a caller comparing a new value
+    // against the stored one byte for byte would read a dequantized reconstruction as a
+    // difference -- or worse, as a match. An SVS index that is quantized should answer the same
+    // way; only an unquantized one can answer truthfully.
+    //
+    // What it unblocks: the no-change-set path in RediSearch (`VectorIndex_HoldsVectors`), which
+    // is what serves JSON writes and background scans. Note that alone is not enough to make an
+    // SVS-backed vector field relabel -- `relabelVector` is also unimplemented for SVS, and both
+    // are needed.
     void getDataByLabel(
         labelType label,
         std::vector<std::vector<svs_details::vecsim_dt<DataType>>> &vectors_output) const override {
-        assert(false && "Not implemented");
+        this->log(VecSimCommonStrings::LOG_DEBUG_STRING,
+                  "getDataByLabel: not implemented for SVS, reporting no stored vectors for "
+                  "label %zu",
+                  static_cast<size_t>(label));
     }
-
-    svs::logging::logger_ptr getLogger() const override { return logger_; }
-#endif
 };
 
 #ifdef BUILD_TESTS

src/VecSim/vec_sim_index.h

@@ -327,6 +327,35 @@ struct VecSimIndexAbstract : public VecSimIndexInterface {
         };
         return info;
     }
+    /**
+     * @brief Get the vector elements stored under a given label, in insertion order.
+     *
+     * A tiered index cannot honour the ordering across its tiers; see
+     * `VecSimTieredIndex::getDataByLabel`.
+     *
+     * Appends nothing when the label is absent, so the output size doubles as the answer to
+     * whether the index holds it -- and likewise for an index type that cannot read its stored
+     * vectors back at all, which is how `SVSIndex` answers.
+     *
+     * Returns ONLY the vector elements, even where the stored vector carries extra
+     * metadata -- with int8_t/uint8_t under cosine, the trailing norm is not included.
+     * Use getStoredVectorDataByLabel() when the complete stored form is wanted.
+     *
+     * The elements are the *stored* ones, i.e. after any insert-time preprocessing —
+     * normalized under cosine, for instance — and not the blob originally handed to
+     * addVector.
+     *
+     * Nothing here dequantizes. A quantized index therefore appends nothing rather than
+     * reinterpreting compression and metadata as values, which reads as "cannot tell". The test
+     * is the index's own `isQuantized`, not a size comparison: an SQ8 element is smaller than the
+     * raw elements only above a certain dimension, and larger below it.
+     *
+     * @param label The label to retrieve vector(s) elements for
+     * @param vectors_output Empty vector to be filled with vector(s)
+     */
+    virtual void getDataByLabel(labelType label,
+                                std::vector<std::vector<DataType>> &vectors_output) const = 0;
+
 #ifdef BUILD_TESTS
     void replacePPContainer(PreprocessorsContainerAbstract *newPPContainer) {
         delete this->preprocessors;
@@ -341,24 +370,6 @@ struct VecSimIndexAbstract : public VecSimIndexInterface {
     // without requiring an actual disk-based index implementation.
     void setIsDiskForTesting(bool v) { this->isDisk = v; }
 
-    /**
-     * @brief Used for testing - get only the vector elements associated with a given label.
-     * This function copies only the vector(s) elements into the output vector,
-     * without any additional metadata that might be stored with the vector.
-     *
-     * Important: This method returns ONLY the vector elements, even if the stored vector contains
-     * additional metadata. For example, with int8_t/uint8_t vectors using cosine similarity,
-     * this method will NOT return the norm that is stored with the vector(s).
-     *
-     * If you need the complete data including any metadata, use getStoredVectorDataByLabel()
-     * instead.
-     *
-     * @param label The label to retrieve vector(s) elements for
-     * @param vectors_output Empty vector to be filled with vector(s)
-     */
-    virtual void getDataByLabel(labelType label,
-                                std::vector<std::vector<DataType>> &vectors_output) const = 0;
-
     /**
      * @brief Used for testing - get the complete raw data associated with a given label.
      * This function returns the ENTIRE vector(s) data as stored in the index, including any

src/VecSim/vec_sim_tiered_index.h

@@ -18,6 +18,11 @@
 
 #include <shared_mutex>
 
+#if HAVE_SVS
+// For the SVS special case in getDataByLabel; remove with it (MOD-17706).
+#include "VecSim/algorithms/svs/svs.h"
+#endif
+
 #define TIERED_LOG this->backendIndex->log
 
 /**
@@ -115,6 +120,73 @@ class VecSimTieredIndex : public VecSimIndexInterface {
                                     VecSimQueryReply_Order order) const;
 
 public:
+    /**
+     * @brief Get the vector elements stored under a label, in insertion order.
+     *
+     * Contract on `VecSimIndexAbstract::getDataByLabel`, including that `vectors_output` arrives
+     * empty, with two caveats a tiered index cannot
+     * avoid, both of which only ever make an equality-testing caller answer "different":
+     *
+     * - The vectors are the buffer's followed by the backend's, which for a multi-value label
+     *   split across the tiers is not insertion order.
+     * - An ingest job inserts into the backend before removing from the buffer, so a vector
+     *   caught inside that window is reported by both tiers and appears twice.
+     *
+     * Which tiers are read follows `getDistanceFrom_Unsafe`: a single-value label found in the
+     * buffer is the whole answer, but a multi-value label's vectors are routinely split across
+     * the tiers while an ingest is pending, so there the backend is read as well. Reading only
+     * the backend, as this used to, reports nothing for a vector written recently enough to
+     * still be buffered -- which is exactly when a document is most likely to be written again.
+     *
+     * `flatIndexGuard` is held across both reads, in the order `relabelVector` and
+     * `acquireSharedLocks` take: it cannot prevent a duplicate, but it does stop the buffer's
+     * copy being removed between them. The backend's own data guard is deliberately not taken
+     * here -- its `getDataByLabel` takes it, because a shared main lock does not exclude an
+     * ingest mutating under `indexDataGuard`. Same division as
+     * `computeUnifiedIndexLabelsSetUnsafe`, which holds the outer locks and lets `getLabelsSet`
+     * take the inner one.
+     */
+    void getDataByLabel(labelType label, std::vector<std::vector<DataType>> &vectors_output) const {
+#ifdef BUILD_TESTS
+        // The base contract asks for an empty output. A caller reusing a vector would otherwise
+        // get this label's vectors appended to the previous label's, with nothing to notice it by.
+        assert(vectors_output.empty() && "getDataByLabel expects an empty output vector");
+#endif
+
+        // A quantized backend cannot report its stored vectors as values -- the stored form is
+        // compression plus metadata, and nothing here dequantizes -- so it would append nothing.
+        bool backend_can_report = true;
+#if HAVE_SVS
+        // TODO(MOD-17706): remove once SVSIndex::getDataByLabel reports real data. Removing it
+        // means deleting this block, the `backend_can_report` flag, and the guarded include of
+        // svs.h, then unwrapping the body below.
+        //
+        // Until then nothing is read at all for an SVS backend: the buffer alone would be a
+        // partial answer for a multi-value label split across the tiers, and a caller cannot tell
+        // a subset from the whole. Skipping the reads also avoids waiting on `mainIndexGuard`
+        // behind an SVS batch update to be told nothing.
+        //
+        // Here rather than as an override in TieredSVSIndex because this method is not virtual:
+        // `VecSimTieredIndex` derives from `VecSimIndexInterface`, which does not declare it, and
+        // callers reach it through a `VecSimTieredIndex *` (RediSearch dynamic_casts to exactly
+        // that), so a derived override would simply not be found. The type test is deliberately
+        // explicit rather than dressed up as a capability: it is a special case, not
+        // architecture.
+        backend_can_report = dynamic_cast<const SVSIndexBase *>(this->backendIndex) == nullptr;
+#endif
+        if (backend_can_report) {
+            std::shared_lock<std::shared_mutex> flat_lock(this->flatIndexGuard);
+            const size_t before_flat = vectors_output.size();
+            this->frontendIndex->getDataByLabel(label, vectors_output);
+            // Whether the buffer held it, measured rather than read off emptiness, so the tier
+            // decision does not depend on an assertion that only exists in test builds.
+            if (this->backendIndex->isMultiValue() || vectors_output.size() == before_flat) {
+                std::shared_lock<std::shared_mutex> main_lock(this->mainIndexGuard);
+                this->backendIndex->getDataByLabel(label, vectors_output);
+            }
+        }
+    }
+
     VecSimTieredIndex(VecSimIndexAbstract<DataType, DistType> *backendIndex_,
                       BruteForceIndex<DataType, DistType> *frontendIndex_,
                       TieredIndexParams tieredParams, std::shared_ptr<VecSimAllocator> allocator)