Skip to content

Commit 8dde00a

Browse files
ofekshenawaofekshenawa
committed
Change to svs-vamana vector type && remove panics from search module
1 parent aae74b2 commit 8dde00a

File tree

2 files changed

+97
-67
lines changed

2 files changed

+97
-67
lines changed

‎search_commands.go

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ func (c cmdable) FTAggregate(ctx context.Context, index string, query string) *M
512512
return cmd
513513
}
514514

515-
func FTAggregateQuery(query string, options *FTAggregateOptions) AggregateQuery {
515+
func FTAggregateQuery(query string, options *FTAggregateOptions) (AggregateQuery, error) {
516516
queryArgs := []interface{}{query}
517517
if options != nil {
518518
if options.Verbatim {
@@ -528,7 +528,7 @@ func FTAggregateQuery(query string, options *FTAggregateOptions) AggregateQuery
528528
}
529529

530530
if options.LoadAll && options.Load != nil {
531-
panic("FT.AGGREGATE: LOADALL and LOAD are mutually exclusive")
531+
return nil, fmt.Errorf("FT.AGGREGATE: LOADALL and LOAD are mutually exclusive")
532532
}
533533
if options.LoadAll {
534534
queryArgs = append(queryArgs, "LOAD", "*")
@@ -584,7 +584,7 @@ func FTAggregateQuery(query string, options *FTAggregateOptions) AggregateQuery
584584
for _, sortBy := range options.SortBy {
585585
sortByOptions = append(sortByOptions, sortBy.FieldName)
586586
if sortBy.Asc && sortBy.Desc {
587-
panic("FT.AGGREGATE: ASC and DESC are mutually exclusive")
587+
return nil, fmt.Errorf("FT.AGGREGATE: ASC and DESC are mutually exclusive")
588588
}
589589
if sortBy.Asc {
590590
sortByOptions = append(sortByOptions, "ASC")
@@ -629,7 +629,7 @@ func FTAggregateQuery(query string, options *FTAggregateOptions) AggregateQuery
629629
queryArgs = append(queryArgs, "DIALECT", 2)
630630
}
631631
}
632-
return queryArgs
632+
return queryArgs, nil
633633
}
634634

635635
func ProcessAggregateResult(data []interface{}) (*FTAggregateResult, error) {
@@ -731,7 +731,9 @@ func (c cmdable) FTAggregateWithArgs(ctx context.Context, index string, query st
731731
args = append(args, "ADDSCORES")
732732
}
733733
if options.LoadAll && options.Load != nil {
734-
panic("FT.AGGREGATE: LOADALL and LOAD are mutually exclusive")
734+
cmd := NewAggregateCmd(ctx, args...)
735+
cmd.SetErr(fmt.Errorf("FT.AGGREGATE: LOADALL and LOAD are mutually exclusive"))
736+
return cmd
735737
}
736738
if options.LoadAll {
737739
args = append(args, "LOAD", "*")
@@ -784,7 +786,9 @@ func (c cmdable) FTAggregateWithArgs(ctx context.Context, index string, query st
784786
for _, sortBy := range options.SortBy {
785787
sortByOptions = append(sortByOptions, sortBy.FieldName)
786788
if sortBy.Asc && sortBy.Desc {
787-
panic("FT.AGGREGATE: ASC and DESC are mutually exclusive")
789+
cmd := NewAggregateCmd(ctx, args...)
790+
cmd.SetErr(fmt.Errorf("FT.AGGREGATE: ASC and DESC are mutually exclusive"))
791+
return cmd
788792
}
789793
if sortBy.Asc {
790794
sortByOptions = append(sortByOptions, "ASC")
@@ -932,7 +936,9 @@ func (c cmdable) FTCreate(ctx context.Context, index string, options *FTCreateOp
932936
args = append(args, "ON", "JSON")
933937
}
934938
if options.OnHash && options.OnJSON {
935-
panic("FT.CREATE: ON HASH and ON JSON are mutually exclusive")
939+
cmd := NewStatusCmd(ctx, args...)
940+
cmd.SetErr(fmt.Errorf("FT.CREATE: ON HASH and ON JSON are mutually exclusive"))
941+
return cmd
936942
}
937943
if options.Prefix != nil {
938944
args = append(args, "PREFIX", len(options.Prefix))
@@ -983,12 +989,16 @@ func (c cmdable) FTCreate(ctx context.Context, index string, options *FTCreateOp
983989
}
984990
}
985991
if schema == nil {
986-
panic("FT.CREATE: SCHEMA is required")
992+
cmd := NewStatusCmd(ctx, args...)
993+
cmd.SetErr(fmt.Errorf("FT.CREATE: SCHEMA is required"))
994+
return cmd
987995
}
988996
args = append(args, "SCHEMA")
989997
for _, schema := range schema {
990998
if schema.FieldName == "" || schema.FieldType == SearchFieldTypeInvalid {
991-
panic("FT.CREATE: SCHEMA FieldName and FieldType are required")
999+
cmd := NewStatusCmd(ctx, args...)
1000+
cmd.SetErr(fmt.Errorf("FT.CREATE: SCHEMA FieldName and FieldType are required"))
1001+
return cmd
9921002
}
9931003
args = append(args, schema.FieldName)
9941004
if schema.As != "" {
@@ -997,7 +1007,9 @@ func (c cmdable) FTCreate(ctx context.Context, index string, options *FTCreateOp
9971007
args = append(args, schema.FieldType.String())
9981008
if schema.VectorArgs != nil {
9991009
if schema.FieldType != SearchFieldTypeVector {
1000-
panic("FT.CREATE: SCHEMA FieldType VECTOR is required for VectorArgs")
1010+
cmd := NewStatusCmd(ctx, args...)
1011+
cmd.SetErr(fmt.Errorf("FT.CREATE: SCHEMA FieldType VECTOR is required for VectorArgs"))
1012+
return cmd
10011013
}
10021014
// Check mutual exclusivity of vector options
10031015
optionCount := 0
@@ -1011,12 +1023,16 @@ func (c cmdable) FTCreate(ctx context.Context, index string, options *FTCreateOp
10111023
optionCount++
10121024
}
10131025
if optionCount != 1 {
1014-
panic("FT.CREATE: SCHEMA VectorArgs must have exactly one of FlatOptions, HNSWOptions, or VamanaOptions")
1026+
cmd := NewStatusCmd(ctx, args...)
1027+
cmd.SetErr(fmt.Errorf("FT.CREATE: SCHEMA VectorArgs must have exactly one of FlatOptions, HNSWOptions, or VamanaOptions"))
1028+
return cmd
10151029
}
10161030
if schema.VectorArgs.FlatOptions != nil {
10171031
args = append(args, "FLAT")
10181032
if schema.VectorArgs.FlatOptions.Type == "" || schema.VectorArgs.FlatOptions.Dim == 0 || schema.VectorArgs.FlatOptions.DistanceMetric == "" {
1019-
panic("FT.CREATE: Type, Dim and DistanceMetric are required for VECTOR FLAT")
1033+
cmd := NewStatusCmd(ctx, args...)
1034+
cmd.SetErr(fmt.Errorf("FT.CREATE: Type, Dim and DistanceMetric are required for VECTOR FLAT"))
1035+
return cmd
10201036
}
10211037
flatArgs := []interface{}{
10221038
"TYPE", schema.VectorArgs.FlatOptions.Type,
@@ -1035,7 +1051,9 @@ func (c cmdable) FTCreate(ctx context.Context, index string, options *FTCreateOp
10351051
if schema.VectorArgs.HNSWOptions != nil {
10361052
args = append(args, "HNSW")
10371053
if schema.VectorArgs.HNSWOptions.Type == "" || schema.VectorArgs.HNSWOptions.Dim == 0 || schema.VectorArgs.HNSWOptions.DistanceMetric == "" {
1038-
panic("FT.CREATE: Type, Dim and DistanceMetric are required for VECTOR HNSW")
1054+
cmd := NewStatusCmd(ctx, args...)
1055+
cmd.SetErr(fmt.Errorf("FT.CREATE: Type, Dim and DistanceMetric are required for VECTOR HNSW"))
1056+
return cmd
10391057
}
10401058
hnswArgs := []interface{}{
10411059
"TYPE", schema.VectorArgs.HNSWOptions.Type,
@@ -1061,9 +1079,11 @@ func (c cmdable) FTCreate(ctx context.Context, index string, options *FTCreateOp
10611079
args = append(args, hnswArgs...)
10621080
}
10631081
if schema.VectorArgs.VamanaOptions != nil {
1064-
args = append(args, "VAMANA")
1082+
args = append(args, "SVS-VAMANA")
10651083
if schema.VectorArgs.VamanaOptions.Type == "" || schema.VectorArgs.VamanaOptions.Dim == 0 || schema.VectorArgs.VamanaOptions.DistanceMetric == "" {
1066-
panic("FT.CREATE: Type, Dim and DistanceMetric are required for VECTOR VAMANA")
1084+
cmd := NewStatusCmd(ctx, args...)
1085+
cmd.SetErr(fmt.Errorf("FT.CREATE: Type, Dim and DistanceMetric are required for VECTOR VAMANA"))
1086+
return cmd
10671087
}
10681088
vamanaArgs := []interface{}{
10691089
"TYPE", schema.VectorArgs.VamanaOptions.Type,
@@ -1097,7 +1117,9 @@ func (c cmdable) FTCreate(ctx context.Context, index string, options *FTCreateOp
10971117
}
10981118
if schema.GeoShapeFieldType != "" {
10991119
if schema.FieldType != SearchFieldTypeGeoShape {
1100-
panic("FT.CREATE: SCHEMA FieldType GEOSHAPE is required for GeoShapeFieldType")
1120+
cmd := NewStatusCmd(ctx, args...)
1121+
cmd.SetErr(fmt.Errorf("FT.CREATE: SCHEMA FieldType GEOSHAPE is required for GeoShapeFieldType"))
1122+
return cmd
11011123
}
11021124
args = append(args, schema.GeoShapeFieldType)
11031125
}
@@ -1255,7 +1277,7 @@ func (c cmdable) FTExplainWithArgs(ctx context.Context, index string, query stri
12551277
// FTExplainCli - Returns the execution plan for a complex query. [Not Implemented]
12561278
// For more information, see https://redis.io/commands/ft.explaincli/
12571279
func (c cmdable) FTExplainCli(ctx context.Context, key, path string) error {
1258-
panic("not implemented")
1280+
return fmt.Errorf("FTExplainCli is not implemented")
12591281
}
12601282

12611283
func parseFTInfo(data map[string]interface{}) (FTInfoResult, error) {
@@ -1810,7 +1832,7 @@ type SearchQuery []interface{}
18101832
// For more information, please refer to the Redis documentation about [FT.SEARCH].
18111833
//
18121834
// [FT.SEARCH]: (https://redis.io/commands/ft.search/)
1813-
func FTSearchQuery(query string, options *FTSearchOptions) SearchQuery {
1835+
func FTSearchQuery(query string, options *FTSearchOptions) (SearchQuery, error) {
18141836
queryArgs := []interface{}{query}
18151837
if options != nil {
18161838
if options.NoContent {
@@ -1890,7 +1912,7 @@ func FTSearchQuery(query string, options *FTSearchOptions) SearchQuery {
18901912
for _, sortBy := range options.SortBy {
18911913
queryArgs = append(queryArgs, sortBy.FieldName)
18921914
if sortBy.Asc && sortBy.Desc {
1893-
panic("FT.SEARCH: ASC and DESC are mutually exclusive")
1915+
return nil, fmt.Errorf("FT.SEARCH: ASC and DESC are mutually exclusive")
18941916
}
18951917
if sortBy.Asc {
18961918
queryArgs = append(queryArgs, "ASC")
@@ -1918,7 +1940,7 @@ func FTSearchQuery(query string, options *FTSearchOptions) SearchQuery {
19181940
queryArgs = append(queryArgs, "DIALECT", 2)
19191941
}
19201942
}
1921-
return queryArgs
1943+
return queryArgs, nil
19221944
}
19231945

19241946
// FTSearchWithArgs - Executes a search query on an index with additional options.
@@ -2007,7 +2029,9 @@ func (c cmdable) FTSearchWithArgs(ctx context.Context, index string, query strin
20072029
for _, sortBy := range options.SortBy {
20082030
args = append(args, sortBy.FieldName)
20092031
if sortBy.Asc && sortBy.Desc {
2010-
panic("FT.SEARCH: ASC and DESC are mutually exclusive")
2032+
cmd := newFTSearchCmd(ctx, options, args...)
2033+
cmd.SetErr(fmt.Errorf("FT.SEARCH: ASC and DESC are mutually exclusive"))
2034+
return cmd
20112035
}
20122036
if sortBy.Asc {
20132037
args = append(args, "ASC")

‎search_test.go

Lines changed: 52 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,8 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
830830
})
831831

832832
It("should return only the base query when options is nil", Label("search", "ftaggregate"), func() {
833-
args := redis.FTAggregateQuery("testQuery", nil)
833+
args, err := redis.FTAggregateQuery("testQuery", nil)
834+
Expect(err).NotTo(HaveOccurred())
834835
Expect(args).To(Equal(redis.AggregateQuery{"testQuery"}))
835836
})
836837

@@ -839,7 +840,8 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
839840
Verbatim: true,
840841
Scorer: "BM25",
841842
}
842-
args := redis.FTAggregateQuery("testQuery", options)
843+
args, err := redis.FTAggregateQuery("testQuery", options)
844+
Expect(err).NotTo(HaveOccurred())
843845
Expect(args[0]).To(Equal("testQuery"))
844846
Expect(args).To(ContainElement("VERBATIM"))
845847
Expect(args).To(ContainElement("SCORER"))
@@ -850,15 +852,17 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
850852
options := &redis.FTAggregateOptions{
851853
AddScores: true,
852854
}
853-
args := redis.FTAggregateQuery("q", options)
855+
args, err := redis.FTAggregateQuery("q", options)
856+
Expect(err).NotTo(HaveOccurred())
854857
Expect(args).To(ContainElement("ADDSCORES"))
855858
})
856859

857860
It("should include LOADALL when LoadAll is true", Label("search", "ftaggregate"), func() {
858861
options := &redis.FTAggregateOptions{
859862
LoadAll: true,
860863
}
861-
args := redis.FTAggregateQuery("q", options)
864+
args, err := redis.FTAggregateQuery("q", options)
865+
Expect(err).NotTo(HaveOccurred())
862866
Expect(args).To(ContainElement("LOAD"))
863867
Expect(args).To(ContainElement("*"))
864868
})
@@ -870,7 +874,8 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
870874
{Field: "field2"},
871875
},
872876
}
873-
args := redis.FTAggregateQuery("q", options)
877+
args, err := redis.FTAggregateQuery("q", options)
878+
Expect(err).NotTo(HaveOccurred())
874879
// Verify LOAD options related arguments
875880
Expect(args).To(ContainElement("LOAD"))
876881
// Check that field names and aliases are present
@@ -883,7 +888,8 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
883888
options := &redis.FTAggregateOptions{
884889
Timeout: 500,
885890
}
886-
args := redis.FTAggregateQuery("q", options)
891+
args, err := redis.FTAggregateQuery("q", options)
892+
Expect(err).NotTo(HaveOccurred())
887893
Expect(args).To(ContainElement("TIMEOUT"))
888894
found := false
889895
for i, a := range args {
@@ -1826,56 +1832,56 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
18261832

18271833
It("should fail FTCreate VECTOR with VAMANA - missing required parameters", Label("search", "ftcreate"), func() {
18281834
// Test missing Type
1829-
Expect(func() {
1830-
client.FTCreate(ctx, "idx1",
1831-
&redis.FTCreateOptions{},
1832-
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{VamanaOptions: &redis.FTVamanaOptions{
1833-
Dim: 2,
1834-
DistanceMetric: "L2",
1835-
}}})
1836-
}).To(Panic())
1835+
cmd := client.FTCreate(ctx, "idx1",
1836+
&redis.FTCreateOptions{},
1837+
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{VamanaOptions: &redis.FTVamanaOptions{
1838+
Dim: 2,
1839+
DistanceMetric: "L2",
1840+
}}})
1841+
Expect(cmd.Err()).To(HaveOccurred())
1842+
Expect(cmd.Err().Error()).To(ContainSubstring("Type, Dim and DistanceMetric are required for VECTOR VAMANA"))
18371843

18381844
// Test missing Dim
1839-
Expect(func() {
1840-
client.FTCreate(ctx, "idx1",
1841-
&redis.FTCreateOptions{},
1842-
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{VamanaOptions: &redis.FTVamanaOptions{
1843-
Type: "FLOAT32",
1844-
DistanceMetric: "L2",
1845-
}}})
1846-
}).To(Panic())
1845+
cmd = client.FTCreate(ctx, "idx1",
1846+
&redis.FTCreateOptions{},
1847+
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{VamanaOptions: &redis.FTVamanaOptions{
1848+
Type: "FLOAT32",
1849+
DistanceMetric: "L2",
1850+
}}})
1851+
Expect(cmd.Err()).To(HaveOccurred())
1852+
Expect(cmd.Err().Error()).To(ContainSubstring("Type, Dim and DistanceMetric are required for VECTOR VAMANA"))
18471853

18481854
// Test missing DistanceMetric
1849-
Expect(func() {
1850-
client.FTCreate(ctx, "idx1",
1851-
&redis.FTCreateOptions{},
1852-
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{VamanaOptions: &redis.FTVamanaOptions{
1853-
Type: "FLOAT32",
1854-
Dim: 2,
1855-
}}})
1856-
}).To(Panic())
1855+
cmd = client.FTCreate(ctx, "idx1",
1856+
&redis.FTCreateOptions{},
1857+
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{VamanaOptions: &redis.FTVamanaOptions{
1858+
Type: "FLOAT32",
1859+
Dim: 2,
1860+
}}})
1861+
Expect(cmd.Err()).To(HaveOccurred())
1862+
Expect(cmd.Err().Error()).To(ContainSubstring("Type, Dim and DistanceMetric are required for VECTOR VAMANA"))
18571863
})
18581864

18591865
It("should fail FTCreate VECTOR with multiple vector options", Label("search", "ftcreate"), func() {
18601866
// Test VAMANA + HNSW
1861-
Expect(func() {
1862-
client.FTCreate(ctx, "idx1",
1863-
&redis.FTCreateOptions{},
1864-
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{
1865-
VamanaOptions: &redis.FTVamanaOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1866-
HNSWOptions: &redis.FTHNSWOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1867-
}})
1868-
}).To(Panic())
1867+
cmd := client.FTCreate(ctx, "idx1",
1868+
&redis.FTCreateOptions{},
1869+
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{
1870+
VamanaOptions: &redis.FTVamanaOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1871+
HNSWOptions: &redis.FTHNSWOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1872+
}})
1873+
Expect(cmd.Err()).To(HaveOccurred())
1874+
Expect(cmd.Err().Error()).To(ContainSubstring("VectorArgs must have exactly one of FlatOptions, HNSWOptions, or VamanaOptions"))
18691875

18701876
// Test VAMANA + FLAT
1871-
Expect(func() {
1872-
client.FTCreate(ctx, "idx1",
1873-
&redis.FTCreateOptions{},
1874-
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{
1875-
VamanaOptions: &redis.FTVamanaOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1876-
FlatOptions: &redis.FTFlatOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1877-
}})
1878-
}).To(Panic())
1877+
cmd = client.FTCreate(ctx, "idx1",
1878+
&redis.FTCreateOptions{},
1879+
&redis.FieldSchema{FieldName: "v", FieldType: redis.SearchFieldTypeVector, VectorArgs: &redis.FTVectorArgs{
1880+
VamanaOptions: &redis.FTVamanaOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1881+
FlatOptions: &redis.FTFlatOptions{Type: "FLOAT32", Dim: 2, DistanceMetric: "L2"},
1882+
}})
1883+
Expect(cmd.Err()).To(HaveOccurred())
1884+
Expect(cmd.Err().Error()).To(ContainSubstring("VectorArgs must have exactly one of FlatOptions, HNSWOptions, or VamanaOptions"))
18791885
})
18801886

18811887
It("should test VAMANA L2 distance metric", Label("search", "ftcreate", "vamana"), func() {

0 commit comments

Comments
 (0)