Skip to content

Commit f22f8e0

Browse files
eharris128shiva-istari
authored andcommitted
test(vector): cover non-positive similar_to end-to-end and pin addPathNode state
Add an integration test (TestSimilarToNonPositiveNeighbors) that fires similar_to with k=0 and k=-1, asserts both return a 'number of neighbors' query error, and then runs a valid similar_to and asserts it succeeds. The trailing valid query is the real crash-regression check: on the pre-fix code the first non-positive query panics and crashes the Alpha. Tighten TestAddPathNodeNonPositiveMaxResults to also pin the post-call state (neighbors truncated to empty, path unchanged) instead of only asserting the absence of a panic.
1 parent 68e9288 commit f22f8e0

2 files changed

Lines changed: 70 additions & 7 deletions

File tree

query/vector/vector_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,63 @@ func TestSimilarToOptionsIntegration(t *testing.T) {
485485
})
486486
}
487487

488+
// TestSimilarToNonPositiveNeighbors is the end-to-end guard for a crash reachable
489+
// from an unvalidated similar_to number-of-neighbors argument. A count <= 0 used to
490+
// panic the query goroutine and take down the Alpha; the worker/task.go boundary
491+
// check must now turn it into an ordinary query error while the server keeps serving.
492+
func TestSimilarToNonPositiveNeighbors(t *testing.T) {
493+
const pred = "vnonpos"
494+
dropPredicate(pred)
495+
t.Cleanup(func() { dropPredicate(pred) })
496+
497+
setSchema(fmt.Sprintf(vectorSchemaWithIndex, pred, "4", "euclidean"))
498+
499+
rdf := `<0x1> <vnonpos> "[0,0]" .
500+
<0x2> <vnonpos> "[1,0]" .
501+
<0x3> <vnonpos> "[2,0]" .`
502+
require.NoError(t, addTriplesToCluster(rdf))
503+
504+
// Both k=0 and k=-1 parse fine through DQL, so they reach the worker guard.
505+
for _, k := range []int{0, -1} {
506+
query := fmt.Sprintf(`{
507+
results(func: similar_to(%s, %d, "[0,0]")) {
508+
uid
509+
}
510+
}`, pred, k)
511+
_, err := processQuery(context.Background(), t, query)
512+
require.Errorf(t, err, "similar_to with k=%d should return an error, not crash", k)
513+
require.ErrorContains(t, err, "number of neighbors")
514+
}
515+
516+
// The load-bearing assertion: a valid query after the bad ones must succeed.
517+
// On the pre-fix code the first non-positive query panics and crashes the
518+
// Alpha, so this only passes if the server stayed up.
519+
query := fmt.Sprintf(`{
520+
results(func: similar_to(%s, 3, "[0,0]")) {
521+
uid
522+
}
523+
}`, pred)
524+
resp := processQueryNoErr(t, query)
525+
526+
var result struct {
527+
Data struct {
528+
Results []struct {
529+
UID string `json:"uid"`
530+
} `json:"results"`
531+
} `json:"data"`
532+
}
533+
require.NoError(t, json.Unmarshal([]byte(resp), &result))
534+
require.Len(t, result.Data.Results, 3)
535+
536+
expected := map[string]struct{}{"0x1": {}, "0x2": {}, "0x3": {}}
537+
for _, r := range result.Data.Results {
538+
_, ok := expected[r.UID]
539+
require.Truef(t, ok, "unexpected uid %s", r.UID)
540+
delete(expected, r.UID)
541+
}
542+
require.Empty(t, expected)
543+
}
544+
488545
func TestVectorInQueryArgument(t *testing.T) {
489546
dropPredicate("vtest")
490547
setSchema(fmt.Sprintf(vectorSchemaWithIndex, "vtest", "4", "euclidean"))

tok/hnsw/persistent_hnsw_test.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
c "github.com/dgraph-io/dgraph/v25/tok/constraints"
1515
"github.com/dgraph-io/dgraph/v25/tok/index"
1616
opt "github.com/dgraph-io/dgraph/v25/tok/options"
17+
"github.com/stretchr/testify/require"
1718
"golang.org/x/exp/slices"
1819
)
1920

@@ -932,13 +933,18 @@ func TestAddPathNodeNonPositiveMaxResults(t *testing.T) {
932933
for _, maxResults := range []int{-1, 0} {
933934
slr := newLayerResult[float64](0)
934935
slr.setFirstPathNode(persistentHeapElement[float64]{value: 0.1, index: 1})
935-
func() {
936-
defer func() {
937-
if r := recover(); r != nil {
938-
t.Fatalf("addPathNode panicked with maxResults=%d: %v", maxResults, r)
939-
}
940-
}()
936+
937+
require.NotPanicsf(t, func() {
941938
slr.addPathNode(persistentHeapElement[float64]{value: 0.2, index: 2}, simType, maxResults)
942-
}()
939+
}, "addPathNode panicked with maxResults=%d", maxResults)
940+
941+
// A non-positive limit clamps effectiveMaxLen to 0, so the neighbor set
942+
// truncates to empty and the bottom-of-path append is skipped. Pin that
943+
// resulting state, not just the absence of a panic: neighbors is empty and
944+
// the path is left exactly as setFirstPathNode set it.
945+
require.Emptyf(t, slr.neighbors,
946+
"neighbors should truncate to empty for maxResults=%d", maxResults)
947+
require.Equalf(t, []uint64{1}, slr.path,
948+
"path should be unchanged for maxResults=%d", maxResults)
943949
}
944950
}

0 commit comments

Comments
 (0)