fix(query): stop sending HNSW params for an unhandled index type (#216)

CHANGELOG.md

@@ -128,6 +128,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`setRadius()` / `setLinear()` / `setUsingRefiner()` failed on a DiskANN field** (#216)
+  - All three threw `query params type does not match the index type of vector field[v], expected DISKANN but got HNSW`. The message blamed HNSW, but the adapter was the one sending HNSW params.
+  - `ensure_query_params_for_field()` had no `IndexType::DISKANN` case, so it fell through to a hardcoded HNSW fallback. The fallback ran even when the index type *had* been resolved, so it converted "I do not handle this index type" into "these are HNSW params", which upstream then rejects.
+  - Radius and linear are supported by the DiskANN core upstream, so they were broken purely by the wrong param type. Both work now; the same holds for `setDiskAnnParams(300)->setRadius(0.5)`, which is the workaround this bug forced on users.
+  - The refiner is an upstream DiskANN limitation, but it now surfaces upstream's own error instead of the misleading type mismatch.
+  - The `default:` branch no longer falls back to HNSW. An index type with no case gets **no** query params, which makes upstream use its own defaults and skip its params-type check — that fails far less often than guessing, and it also covers index types added upstream after this adapter was written. The trade-off: radius/linear/refiner are not applied in that case, so each new index type needs its own branch. The group-by switch got the same treatment.
+  - Test: `tests/bug_0057.phpt`. Verified that HNSW, IVF, Flat and Vamana still filter by radius correctly.
+
 - **A failed `destroy()` left the PHP object with a dangling handle** (#222)
   - `zvec_collection_destroy()` erased the handle from the collections registry even when upstream returned an error, deleting the C++ `Collection` while the PHP object kept the raw pointer. The next method call on it operated on freed memory — reachable by destroying a read-only collection, which upstream rejects with `INVALID_ARGUMENT`, and later by destroying while a document iterator is open.
   - The registry entry is now erased only on success. On failure upstream leaves the collection untouched, so the handle stays valid and the object remains open and usable.

ffi/zvec_ffi.cc

@@ -2627,13 +2627,28 @@ static void ensure_query_params_for_field(Collection* c, SearchQuery& query, con
                     case IndexType::VAMANA:
                         query.target_.query_params_ = std::make_shared<VamanaQueryParams>(200, holder->radius_, holder->is_linear_, holder->is_using_refiner_);
                         return;
+                    case IndexType::DISKANN:
+                        // DiskAnnQueryParams only takes list_size; radius and the
+                        // flags come from the common QueryParams base.
+                        query.target_.query_params_ = std::make_shared<DiskAnnQueryParams>();
+                        query.target_.query_params_->set_radius(holder->radius_);
+                        query.target_.query_params_->set_is_linear(holder->is_linear_);
+                        query.target_.query_params_->set_is_using_refiner(holder->is_using_refiner_);
+                        return;
                     default:
-                        break;
+                        // An index type we do not handle. Leaving query_params_ null
+                        // lets upstream use its own defaults and skip its params-type
+                        // check, which fails far less often than guessing HNSW: the
+                        // guess was rejected outright for DISKANN before this case
+                        // existed, and for any index type added upstream later.
+                        // Trade-off: radius/linear/refiner are not applied in that
+                        // case. Each new index type needs its own branch here.
+                        return;
                 }
             }
         }
-        // Fallback: use HNSW params if can't determine index type
-        query.target_.query_params_ = std::make_shared<HnswQueryParams>(200, holder->radius_, holder->is_linear_, holder->is_using_refiner_);
+        // Could not read the schema or the field. Same reasoning as above: no
+        // params beats the wrong ones.
     }
 }
 
@@ -2699,8 +2714,16 @@ zvec_status_t zvec_collection_group_by_query_vector(zvec_collection_t coll, cons
                     case IndexType::VAMANA:
                         holder->query.target_.query_params_ = std::make_shared<VamanaQueryParams>(200, holder->radius_, holder->is_linear_, holder->is_using_refiner_);
                         break;
+                    case IndexType::DISKANN:
+                        holder->query.target_.query_params_ = std::make_shared<DiskAnnQueryParams>();
+                        holder->query.target_.query_params_->set_radius(holder->radius_);
+                        holder->query.target_.query_params_->set_is_linear(holder->is_linear_);
+                        holder->query.target_.query_params_->set_is_using_refiner(holder->is_using_refiner_);
+                        break;
                     default:
-                        holder->query.target_.query_params_ = std::make_shared<HnswQueryParams>(200, holder->radius_, holder->is_linear_, holder->is_using_refiner_);
+                        // See ensure_query_params_for_field(): an unhandled index
+                        // type gets no params rather than wrong ones, so the user
+                        // sees upstream's real "not supported" message.
                         break;
                 }
             }

tests/bug_0057.phpt

@@ -0,0 +1,92 @@
+--TEST--
+Bug 0057: queryVector() with radius/linear/refiner on a DiskANN field sent HNSW params
+--SKIPIF--
+<?php if (!extension_loaded('ffi')) die('skip FFI extension not available'); ?>
+--FILE--
+<?php
+/**
+ * Bug reproduction: radius/linear/refiner on a DiskANN field failed.
+ *
+ * Expected: radius and linear work; the refiner, which DiskANN does not
+ *           support upstream, reports that rather than a params-type mismatch.
+ * Actual:   every one of the three failed with
+ *           "query params type does not match the index type of vector
+ *           field[v], expected DISKANN but got HNSW".
+ *
+ * Cause:    ensure_query_params_for_field() had no IndexType::DISKANN case, so
+ *           it fell through to a hardcoded HNSW fallback. Upstream then
+ *           rejected the params because their type did not match the field's
+ *           index type. The fallback ran even when the index type *was*
+ *           resolved, so the error blamed HNSW for a field that is DiskANN.
+ *
+ *           Radius and linear are genuinely supported by the DiskANN core, so
+ *           they were broken purely by the wrong param type. The refiner is
+ *           an upstream limitation, but it should say so.
+ *
+ * Status: Fixed -- a DISKANN case builds DiskAnnQueryParams, and an index
+ *         type with no case gets no params at all instead of wrong ones.
+ *
+ * Location: ffi/zvec_ffi.cc, ensure_query_params_for_field() and the group-by
+ *           switch in zvec_collection_group_by_query_vector().
+ */
+
+declare(strict_types=1);
+require_once __DIR__ . '/../src/ZVec.php';
+// LOG_FATAL: the refiner case is *expected* to fail, and upstream logs it at
+// ERROR on stderr, which would otherwise land in the expected output.
+ZVec::init(logType: ZVec::LOG_CONSOLE, logLevel: ZVec::LOG_FATAL);
+
+$path = __DIR__ . '/../test_dbs/bug_0057_' . uniqid();
+
+try {
+    $schema = new ZVecSchema('bug_0057');
+    $schema->addVectorFp32('v', dimension: 4, metricType: ZVecSchema::METRIC_L2);
+    $c = ZVec::create($path, $schema);
+    $c->createIndex('v', ZVecIndexParams::forDiskAnn(
+        metricType: ZVecSchema::METRIC_L2,
+        maxDegree: 32,
+        listSize: 100,
+    ));
+    foreach ([['doc1', [1.0, 0.0, 0.0, 0.0]], ['doc2', [0.9, 0.1, 0.0, 0.0]], ['doc3', [0.0, 1.0, 0.0, 0.0]]] as [$pk, $vec]) {
+        $c->insert((new ZVecDoc($pk))->setVectorFp32('v', $vec));
+    }
+    $c->optimize();
+
+    $cases = [
+        // doc3 is at squared L2 distance 2.0, so a radius of 0.5 excludes it.
+        'radius only' => static fn(ZVecVectorQuery $q) => $q->setRadius(0.5),
+        'linear only' => static fn(ZVecVectorQuery $q) => $q->setLinear(true),
+        'explicit+radius' => static fn(ZVecVectorQuery $q) => $q->setDiskAnnParams(300)->setRadius(0.5),
+    ];
+    foreach ($cases as $name => $apply) {
+        $q = (new ZVecVectorQuery('v', [1.0, 0.0, 0.0, 0.0]))->setTopk(3);
+        $apply($q);
+        try {
+            $pks = array_map(static fn(ZVecDoc $d): string => $d->getPk(), $c->queryVector($q));
+            echo "$name: " . implode(',', $pks) . "\n";
+        } catch (ZVecException $e) {
+            echo "$name: ERROR code={$e->getCode()} {$e->getMessage()}\n";
+        }
+    }
+
+    // The refiner is not supported by the DiskANN core upstream. What matters
+    // is that the failure is now upstream's own, not "expected DISKANN but
+    // got HNSW" -- that message blamed the wrong thing entirely.
+    $refiner = (new ZVecVectorQuery('v', [1.0, 0.0, 0.0, 0.0]))->setTopk(3)->setUsingRefiner(true);
+    try {
+        $c->queryVector($refiner);
+        echo "refiner: accepted\n";
+    } catch (ZVecException $e) {
+        echo 'refiner: ' . (str_contains($e->getMessage(), 'expected DISKANN but got HNSW') ? 'WRONG PARAMS' : 'upstream error') . "\n";
+    }
+
+    $c->close();
+} finally {
+    exec('rm -rf ' . escapeshellarg($path));
+}
+?>
+--EXPECT--
+radius only: doc1,doc2
+linear only: doc1,doc2,doc3
+explicit+radius: doc1,doc2
+refiner: upstream error