diff --git a/go/vt/vtgate/engine/delete_test.go b/go/vt/vtgate/engine/delete_test.go index 4cd0efcde67..47ecf8306af 100644 --- a/go/vt/vtgate/engine/delete_test.go +++ b/go/vt/vtgate/engine/delete_test.go @@ -64,7 +64,7 @@ func TestDeleteUnsharded(t *testing.T) { } func TestDeleteEqual(t *testing.T) { - vindex, _ := vindexes.NewHash("", nil) + vindex, _ := vindexes.CreateVindex("hash", "", nil) del := &Delete{ DML: &DML{ RoutingParameters: &RoutingParameters{ @@ -96,7 +96,7 @@ func TestDeleteEqual(t *testing.T) { } func TestDeleteEqualMultiCol(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) del := &Delete{ DML: &DML{ RoutingParameters: &RoutingParameters{ @@ -128,7 +128,7 @@ func TestDeleteEqualMultiCol(t *testing.T) { } func TestDeleteEqualNoRoute(t *testing.T) { - vindex, _ := vindexes.NewLookupUnique("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup_unique", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -160,7 +160,7 @@ func TestDeleteEqualNoRoute(t *testing.T) { func TestDeleteEqualNoScatter(t *testing.T) { t.Skip("planner does not produces this plan anymore") - vindex, _ := vindexes.NewLookupUnique("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup_unique", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -555,7 +555,7 @@ func TestDeleteInChangedVindexMultiCol(t *testing.T) { } func TestDeleteEqualSubshard(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) del := &Delete{ DML: &DML{ RoutingParameters: &RoutingParameters{ diff --git a/go/vt/vtgate/engine/plan_description_test.go b/go/vt/vtgate/engine/plan_description_test.go index 0d985b9b606..b986cea59cf 100644 --- a/go/vt/vtgate/engine/plan_description_test.go +++ b/go/vt/vtgate/engine/plan_description_test.go @@ -50,7 +50,7 @@ func TestCreateRoutePlanDescription(t *testing.T) { } func createRoute() *Route { - hash, _ := vindexes.NewHash("vindex name", nil) + hash, _ := vindexes.CreateVindex("hash", "vindex name", nil) return &Route{ RoutingParameters: &RoutingParameters{ Opcode: Scatter, diff --git a/go/vt/vtgate/engine/route_test.go b/go/vt/vtgate/engine/route_test.go index ef0b494e999..bb0def01169 100644 --- a/go/vt/vtgate/engine/route_test.go +++ b/go/vt/vtgate/engine/route_test.go @@ -236,7 +236,7 @@ func TestSelectScatter(t *testing.T) { } func TestSelectEqualUnique(t *testing.T) { - vindex, _ := vindexes.NewHash("", nil) + vindex, _ := vindexes.CreateVindex("hash", "", nil) sel := NewRoute( EqualUnique, &vindexes.Keyspace{ @@ -274,7 +274,7 @@ func TestSelectEqualUnique(t *testing.T) { } func TestSelectNone(t *testing.T) { - vindex, _ := vindexes.NewHash("", nil) + vindex, _ := vindexes.CreateVindex("hash", "", nil) sel := NewRoute( None, &vindexes.Keyspace{ @@ -325,7 +325,7 @@ func TestSelectNone(t *testing.T) { } func TestSelectEqualUniqueScatter(t *testing.T) { - vindex, _ := vindexes.NewLookupUnique("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup_unique", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -368,7 +368,7 @@ func TestSelectEqualUniqueScatter(t *testing.T) { } func TestSelectEqual(t *testing.T) { - vindex, _ := vindexes.NewLookup("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -421,7 +421,7 @@ func TestSelectEqual(t *testing.T) { } func TestSelectEqualNoRoute(t *testing.T) { - vindex, _ := vindexes.NewLookupUnique("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup_unique", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -485,7 +485,7 @@ func TestSelectEqualNoRoute(t *testing.T) { } func TestINUnique(t *testing.T) { - vindex, _ := vindexes.NewHash("", nil) + vindex, _ := vindexes.CreateVindex("hash", "", nil) sel := NewRoute( IN, &vindexes.Keyspace{ @@ -530,7 +530,7 @@ func TestINUnique(t *testing.T) { } func TestINNonUnique(t *testing.T) { - vindex, _ := vindexes.NewLookup("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -597,7 +597,7 @@ func TestINNonUnique(t *testing.T) { } func TestMultiEqual(t *testing.T) { - vindex, _ := vindexes.NewHash("", nil) + vindex, _ := vindexes.CreateVindex("hash", "", nil) sel := NewRoute( MultiEqual, &vindexes.Keyspace{ @@ -640,7 +640,7 @@ func TestMultiEqual(t *testing.T) { } func TestSelectLike(t *testing.T) { - subshard, _ := vindexes.NewCFC("cfc", map[string]string{"hash": "md5", "offsets": "[1,2]"}) + subshard, _ := vindexes.CreateVindex("cfc", "cfc", map[string]string{"hash": "md5", "offsets": "[1,2]"}) vindex := subshard.(*vindexes.CFC).PrefixVindex() vc := &loggingVCursor{ // we have shards '-0c80', '0c80-0d', '0d-40', '40-80', '80-' @@ -816,7 +816,7 @@ func TestSelectReference(t *testing.T) { } func TestRouteGetFields(t *testing.T) { - vindex, _ := vindexes.NewLookupUnique("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup_unique", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -1452,7 +1452,7 @@ func TestExecFail(t *testing.T) { } func TestSelectEqualUniqueMultiColumnVindex(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) sel := NewRoute( EqualUnique, &vindexes.Keyspace{ @@ -1491,7 +1491,7 @@ func TestSelectEqualUniqueMultiColumnVindex(t *testing.T) { } func TestSelectEqualMultiColumnVindex(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) vc := &loggingVCursor{ shards: []string{"-20", "20-"}, shardForKsid: []string{"-20", "20-"}, @@ -1528,7 +1528,7 @@ func TestSelectEqualMultiColumnVindex(t *testing.T) { } func TestINMultiColumnVindex(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) sel := NewRoute( IN, &vindexes.Keyspace{ @@ -1574,7 +1574,7 @@ func TestINMultiColumnVindex(t *testing.T) { } func TestINMixedMultiColumnComparision(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) sel := NewRoute( IN, &vindexes.Keyspace{ @@ -1617,7 +1617,7 @@ func TestINMixedMultiColumnComparision(t *testing.T) { } func TestMultiEqualMultiCol(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) sel := NewRoute( MultiEqual, &vindexes.Keyspace{Name: "ks", Sharded: true}, diff --git a/go/vt/vtgate/engine/update_test.go b/go/vt/vtgate/engine/update_test.go index e87919611ba..d4fab943a00 100644 --- a/go/vt/vtgate/engine/update_test.go +++ b/go/vt/vtgate/engine/update_test.go @@ -68,7 +68,7 @@ func TestUpdateUnsharded(t *testing.T) { } func TestUpdateEqual(t *testing.T) { - vindex, _ := vindexes.NewHash("", nil) + vindex, _ := vindexes.CreateVindex("hash", "", nil) upd := &Update{ DML: &DML{ RoutingParameters: &RoutingParameters{ @@ -99,7 +99,7 @@ func TestUpdateEqual(t *testing.T) { } func TestUpdateEqualMultiCol(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) upd := &Update{ DML: &DML{ RoutingParameters: &RoutingParameters{ @@ -125,7 +125,7 @@ func TestUpdateEqualMultiCol(t *testing.T) { } func TestUpdateScatter(t *testing.T) { - vindex, _ := vindexes.NewHash("", nil) + vindex, _ := vindexes.CreateVindex("hash", "", nil) upd := &Update{ DML: &DML{ RoutingParameters: &RoutingParameters{ @@ -178,7 +178,7 @@ func TestUpdateScatter(t *testing.T) { } func TestUpdateEqualNoRoute(t *testing.T) { - vindex, _ := vindexes.NewLookupUnique("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup_unique", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -210,7 +210,7 @@ func TestUpdateEqualNoRoute(t *testing.T) { func TestUpdateEqualNoScatter(t *testing.T) { t.Skip("planner does not produces this plan anymore") - vindex, _ := vindexes.NewLookupUnique("", map[string]string{ + vindex, _ := vindexes.CreateVindex("lookup_unique", "", map[string]string{ "table": "lkp", "from": "from", "to": "toc", @@ -890,7 +890,7 @@ func TestUpdateInChangedVindexMultiCol(t *testing.T) { } func TestUpdateEqualSubshard(t *testing.T) { - vindex, _ := vindexes.NewRegionExperimental("", map[string]string{"region_bytes": "1"}) + vindex, _ := vindexes.CreateVindex("region_experimental", "", map[string]string{"region_bytes": "1"}) upd := &Update{ DML: &DML{ RoutingParameters: &RoutingParameters{ diff --git a/go/vt/vtgate/planbuilder/from.go b/go/vt/vtgate/planbuilder/from.go index 59df24146fb..669b92346e7 100644 --- a/go/vt/vtgate/planbuilder/from.go +++ b/go/vt/vtgate/planbuilder/from.go @@ -285,7 +285,7 @@ func (pb *primitiveBuilder) buildTablePrimitive(tableExpr *sqlparser.AliasedTabl // Use the Binary vindex, which is the identity function // for keyspace id. eroute = engine.NewSimpleRoute(engine.EqualUnique, vschemaTable.Keyspace) - vindex, _ = vindexes.NewBinary("binary", nil) + vindex, _ = vindexes.CreateVindex("binary", "binary", nil) eroute.Vindex = vindex lit := evalengine.NewLiteralString(vschemaTable.Pinned, collations.TypedCollation{}) eroute.Values = []evalengine.Expr{lit} diff --git a/go/vt/vtgate/planbuilder/operators/sharded_routing.go b/go/vt/vtgate/planbuilder/operators/sharded_routing.go index cef371824cb..e565684507e 100644 --- a/go/vt/vtgate/planbuilder/operators/sharded_routing.go +++ b/go/vt/vtgate/planbuilder/operators/sharded_routing.go @@ -64,7 +64,7 @@ func newShardedRouting(vtable *vindexes.Table, id semantics.TableSet) Routing { // Use the Binary vindex, which is the identity function // for keyspace id. routing.RouteOpCode = engine.EqualUnique - vindex, _ := vindexes.NewBinary("binary", nil) + vindex, _ := vindexes.CreateVindex("binary", "binary", nil) routing.Selected = &VindexOption{ Ready: true, Values: []evalengine.Expr{evalengine.NewLiteralString(vtable.Pinned, collations.TypedCollation{})}, diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 6e20496246d..c43498aecdb 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -1186,7 +1186,7 @@ func TestScopingWVindexTables(t *testing.T) { t.Run(query.query, func(t *testing.T) { parse, err := sqlparser.Parse(query.query) require.NoError(t, err) - hash, _ := vindexes.NewHash("user_index", nil) + hash, _ := vindexes.CreateVindex("hash", "user_index", nil) st, err := Analyze(parse, "user", &FakeSI{ Tables: map[string]*vindexes.Table{ "t": {Name: sqlparser.NewIdentifierCS("t")}, diff --git a/go/vt/vtgate/vindexes/binary.go b/go/vt/vtgate/vindexes/binary.go index d4487ee84ab..b78451ca1fb 100644 --- a/go/vt/vtgate/vindexes/binary.go +++ b/go/vt/vtgate/vindexes/binary.go @@ -26,19 +26,24 @@ import ( ) var ( - _ SingleColumn = (*Binary)(nil) - _ Reversible = (*Binary)(nil) - _ Hashing = (*Binary)(nil) + _ SingleColumn = (*Binary)(nil) + _ Reversible = (*Binary)(nil) + _ Hashing = (*Binary)(nil) + _ ParamValidating = (*Binary)(nil) ) // Binary is a vindex that converts binary bits to a keyspace id. type Binary struct { - name string + name string + unknownParams []string } -// NewBinary creates a new Binary. -func NewBinary(name string, _ map[string]string) (Vindex, error) { - return &Binary{name: name}, nil +// newBinary creates a new Binary. +func newBinary(name string, params map[string]string) (Vindex, error) { + return &Binary{ + name: name, + unknownParams: FindUnknownParams(params, nil), + }, nil } // String returns the name of the vindex. @@ -103,6 +108,11 @@ func (*Binary) ReverseMap(_ VCursor, ksids [][]byte) ([]sqltypes.Value, error) { return reverseIds, nil } +// UnknownParams implements the ParamValidating interface. +func (vind *Binary) UnknownParams() []string { + return vind.unknownParams +} + func init() { - Register("binary", NewBinary) + Register("binary", newBinary) } diff --git a/go/vt/vtgate/vindexes/binary_test.go b/go/vt/vtgate/vindexes/binary_test.go index a1675b4b44d..27ae6ceca11 100644 --- a/go/vt/vtgate/vindexes/binary_test.go +++ b/go/vt/vtgate/vindexes/binary_test.go @@ -24,7 +24,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -34,15 +33,58 @@ import ( var binOnlyVindex SingleColumn func init() { - vindex, _ := CreateVindex("binary", "binary_varchar", nil) + vindex, err := CreateVindex("binary", "binary_varchar", nil) + if err != nil { + panic(err) + } binOnlyVindex = vindex.(SingleColumn) } -func TestBinaryInfo(t *testing.T) { - assert.Equal(t, 0, binOnlyVindex.Cost()) - assert.Equal(t, "binary_varchar", binOnlyVindex.String()) - assert.True(t, binOnlyVindex.IsUnique()) - assert.False(t, binOnlyVindex.NeedsVCursor()) +func binaryCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "binary", + vindexName: "binary", + vindexParams: vindexParams, + + expectCost: 0, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "binary", + expectUnknownParams: expectUnknownParams, + } +} + +func TestBinaryCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + binaryCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + binaryCreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + binaryCreateVindexTestCase( + "unknown params", + map[string]string{"hello": "world"}, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestBinaryMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/binarymd5.go b/go/vt/vtgate/vindexes/binarymd5.go index 49d823a7ed7..2dcaefb7a0d 100644 --- a/go/vt/vtgate/vindexes/binarymd5.go +++ b/go/vt/vtgate/vindexes/binarymd5.go @@ -26,18 +26,23 @@ import ( ) var ( - _ SingleColumn = (*BinaryMD5)(nil) - _ Hashing = (*BinaryMD5)(nil) + _ SingleColumn = (*BinaryMD5)(nil) + _ Hashing = (*BinaryMD5)(nil) + _ ParamValidating = (*BinaryMD5)(nil) ) // BinaryMD5 is a vindex that hashes binary bits to a keyspace id. type BinaryMD5 struct { - name string + name string + params map[string]string } -// NewBinaryMD5 creates a new BinaryMD5. -func NewBinaryMD5(name string, _ map[string]string) (Vindex, error) { - return &BinaryMD5{name: name}, nil +// newBinaryMD5 creates a new BinaryMD5. +func newBinaryMD5(name string, params map[string]string) (Vindex, error) { + return &BinaryMD5{ + name: name, + params: params, + }, nil } // String returns the name of the vindex. @@ -94,11 +99,16 @@ func (vind *BinaryMD5) Hash(id sqltypes.Value) ([]byte, error) { return vMD5Hash(idBytes), nil } +// UnknownParams implements the ParamValidating interface. +func (vind *BinaryMD5) UnknownParams() []string { + return FindUnknownParams(vind.params, nil) +} + func vMD5Hash(source []byte) []byte { sum := md5.Sum(source) return sum[:] } func init() { - Register("binary_md5", NewBinaryMD5) + Register("binary_md5", newBinaryMD5) } diff --git a/go/vt/vtgate/vindexes/binarymd5_test.go b/go/vt/vtgate/vindexes/binarymd5_test.go index c3c5dccb0aa..2c25bc794d4 100644 --- a/go/vt/vtgate/vindexes/binarymd5_test.go +++ b/go/vt/vtgate/vindexes/binarymd5_test.go @@ -23,7 +23,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -33,15 +32,58 @@ import ( var binVindex SingleColumn func init() { - vindex, _ := CreateVindex("binary_md5", "binary_md5_varchar", nil) + vindex, err := CreateVindex("binary_md5", "binary_md5_varchar", nil) + if err != nil { + panic(err) + } binVindex = vindex.(SingleColumn) } -func TestBinaryMD5Info(t *testing.T) { - assert.Equal(t, 1, binVindex.Cost()) - assert.Equal(t, "binary_md5_varchar", binVindex.String()) - assert.True(t, binVindex.IsUnique()) - assert.False(t, binVindex.NeedsVCursor()) +func binaryMD5CreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "binary_md5", + vindexName: "binary_md5", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "binary_md5", + expectUnknownParams: expectUnknownParams, + } +} + +func TestBinaryMD5CreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + binaryMD5CreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + binaryMD5CreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + binaryMD5CreateVindexTestCase( + "unknown params", + map[string]string{"hello": "world"}, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestBinaryMD5Map(t *testing.T) { @@ -134,3 +176,17 @@ func benchmarkMD5HashBytes(b *testing.B, input []byte) { sinkMD5 = vMD5Hash(input) } } + +func TestCreateVindexBinaryMD5Params(t *testing.T) { + vindex, err := CreateVindex("binary_md5", "binary_md5", nil) + require.NotNil(t, vindex) + unknownParams := vindex.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + require.NoError(t, err) + + vindex, err = CreateVindex("binary_md5", "binary_md5", map[string]string{"hello": "world"}) + require.NotNil(t, vindex) + unknownParams = vindex.(ParamValidating).UnknownParams() + require.Len(t, unknownParams, 1) + require.NoError(t, err) +} diff --git a/go/vt/vtgate/vindexes/cached_size.go b/go/vt/vtgate/vindexes/cached_size.go index 55bbd44ea2d..492180e2d09 100644 --- a/go/vt/vtgate/vindexes/cached_size.go +++ b/go/vt/vtgate/vindexes/cached_size.go @@ -49,22 +49,46 @@ func (cached *Binary) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } + +//go:nocheckptr func (cached *BinaryMD5) CachedSize(alloc bool) int64 { if cached == nil { return int64(0) } size := int64(0) if alloc { - size += int64(16) + size += int64(24) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field params map[string]string + if cached.params != nil { + size += int64(48) + hmap := reflect.ValueOf(cached.params) + numBuckets := int(math.Pow(2, float64((*(*uint8)(unsafe.Pointer(hmap.Pointer() + uintptr(9))))))) + numOldBuckets := (*(*uint16)(unsafe.Pointer(hmap.Pointer() + uintptr(10)))) + size += hack.RuntimeAllocSize(int64(numOldBuckets * 272)) + if len(cached.params) > 0 || numBuckets > 1 { + size += hack.RuntimeAllocSize(int64(numBuckets * 272)) + } + for k, v := range cached.params { + size += hack.RuntimeAllocSize(int64(len(k))) + size += hack.RuntimeAllocSize(int64(len(v))) + } + } return size } func (cached *CFC) CachedSize(alloc bool) int64 { @@ -126,10 +150,17 @@ func (cached *ConsistentLookup) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(8) + size += int64(32) } // field clCommon *vitess.io/vitess/go/vt/vtgate/vindexes.clCommon size += cached.clCommon.CachedSize(true) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *ConsistentLookupUnique) CachedSize(alloc bool) int64 { @@ -138,10 +169,17 @@ func (cached *ConsistentLookupUnique) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(8) + size += int64(32) } // field clCommon *vitess.io/vitess/go/vt/vtgate/vindexes.clCommon size += cached.clCommon.CachedSize(true) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *Hash) CachedSize(alloc bool) int64 { @@ -150,10 +188,17 @@ func (cached *Hash) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *Keyspace) CachedSize(alloc bool) int64 { @@ -174,12 +219,19 @@ func (cached *LookupHash) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(176) + size += int64(192) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) // field lkp vitess.io/vitess/go/vt/vtgate/vindexes.lookupInternal size += cached.lkp.CachedSize(false) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *LookupHashUnique) CachedSize(alloc bool) int64 { @@ -188,12 +240,19 @@ func (cached *LookupHashUnique) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(176) + size += int64(192) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) // field lkp vitess.io/vitess/go/vt/vtgate/vindexes.lookupInternal size += cached.lkp.CachedSize(false) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *LookupNonUnique) CachedSize(alloc bool) int64 { @@ -202,12 +261,19 @@ func (cached *LookupNonUnique) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(176) + size += int64(192) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) // field lkp vitess.io/vitess/go/vt/vtgate/vindexes.lookupInternal size += cached.lkp.CachedSize(false) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *LookupUnicodeLooseMD5Hash) CachedSize(alloc bool) int64 { @@ -216,12 +282,19 @@ func (cached *LookupUnicodeLooseMD5Hash) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(176) + size += int64(192) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) // field lkp vitess.io/vitess/go/vt/vtgate/vindexes.lookupInternal size += cached.lkp.CachedSize(false) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *LookupUnicodeLooseMD5HashUnique) CachedSize(alloc bool) int64 { @@ -230,12 +303,19 @@ func (cached *LookupUnicodeLooseMD5HashUnique) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(176) + size += int64(192) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) // field lkp vitess.io/vitess/go/vt/vtgate/vindexes.lookupInternal size += cached.lkp.CachedSize(false) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *LookupUnique) CachedSize(alloc bool) int64 { @@ -244,12 +324,19 @@ func (cached *LookupUnique) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(176) + size += int64(192) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) // field lkp vitess.io/vitess/go/vt/vtgate/vindexes.lookupInternal size += cached.lkp.CachedSize(false) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } @@ -299,10 +386,17 @@ func (cached *Null) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *Numeric) CachedSize(alloc bool) int64 { @@ -311,10 +405,17 @@ func (cached *Numeric) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } @@ -325,7 +426,7 @@ func (cached *NumericStaticMap) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(48) + size += int64(64) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) @@ -344,6 +445,13 @@ func (cached *NumericStaticMap) CachedSize(alloc bool) int64 { size += hack.RuntimeAllocSize(int64(numBuckets * 144)) } } + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *RegionExperimental) CachedSize(alloc bool) int64 { @@ -352,10 +460,17 @@ func (cached *RegionExperimental) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(24) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } @@ -366,7 +481,7 @@ func (cached *RegionJSON) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(32) + size += int64(64) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) @@ -384,6 +499,13 @@ func (cached *RegionJSON) CachedSize(alloc bool) int64 { size += hack.RuntimeAllocSize(int64(len(k))) } } + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *ReverseBits) CachedSize(alloc bool) int64 { @@ -392,10 +514,17 @@ func (cached *ReverseBits) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *Source) CachedSize(alloc bool) int64 { @@ -485,10 +614,17 @@ func (cached *UnicodeLooseMD5) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *UnicodeLooseXXHash) CachedSize(alloc bool) int64 { @@ -497,10 +633,17 @@ func (cached *UnicodeLooseXXHash) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *XXHash) CachedSize(alloc bool) int64 { @@ -509,10 +652,17 @@ func (cached *XXHash) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(16) + size += int64(48) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *cfcCommon) CachedSize(alloc bool) int64 { @@ -521,7 +671,7 @@ func (cached *cfcCommon) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(48) + size += int64(80) } // field name string size += hack.RuntimeAllocSize(int64(len(cached.name))) @@ -529,6 +679,13 @@ func (cached *cfcCommon) CachedSize(alloc bool) int64 { { size += hack.RuntimeAllocSize(int64(cap(cached.offsets)) * int64(8)) } + // field unknownParams []string + { + size += hack.RuntimeAllocSize(int64(cap(cached.unknownParams)) * int64(16)) + for _, elem := range cached.unknownParams { + size += hack.RuntimeAllocSize(int64(len(elem))) + } + } return size } func (cached *clCommon) CachedSize(alloc bool) int64 { diff --git a/go/vt/vtgate/vindexes/cfc.go b/go/vt/vtgate/vindexes/cfc.go index 0be28f96bc9..af269b1a0d9 100644 --- a/go/vt/vtgate/vindexes/cfc.go +++ b/go/vt/vtgate/vindexes/cfc.go @@ -28,6 +28,20 @@ import ( "vitess.io/vitess/go/vt/vterrors" ) +const ( + cfcParamHash = "hash" + cfcParamOffsets = "offsets" +) + +var ( + _ ParamValidating = (*CFC)(nil) + + cfcParams = []string{ + cfcParamHash, + cfcParamOffsets, + } +) + // CFC is Concatenated Fixed-width Composite Vindex. // // The purpose of this vindex is to shard the rows based on the prefix of @@ -94,15 +108,17 @@ type CFC struct { } type cfcCommon struct { - name string - hash func([]byte) []byte - offsets []int + name string + hash func([]byte) []byte + offsets []int + unknownParams []string } -// NewCFC creates a new CFC vindex -func NewCFC(name string, params map[string]string) (Vindex, error) { +// newCFC creates a new CFC vindex +func newCFC(name string, params map[string]string) (Vindex, error) { ss := &cfcCommon{ - name: name, + name: name, + unknownParams: FindUnknownParams(params, cfcParams), } cfc := &CFC{ cfcCommon: ss, @@ -113,7 +129,7 @@ func NewCFC(name string, params map[string]string) (Vindex, error) { return cfc, nil } - switch h := params["hash"]; h { + switch h := params[cfcParamHash]; h { case "": return cfc, nil case "md5": @@ -125,7 +141,7 @@ func NewCFC(name string, params map[string]string) (Vindex, error) { } var offsets []int - if p := params["offsets"]; p == "" { + if p := params[cfcParamOffsets]; p == "" { return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "CFC vindex requires offsets when hash is defined") } else if err := json.Unmarshal([]byte(p), &offsets); err != nil || !validOffsets(offsets) { return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets %s to CFC vindex %s. expected sorted positive ints in brackets", p, name) @@ -231,6 +247,11 @@ func (vind *cfcCommon) verify(ids []sqltypes.Value, ksids [][]byte) ([]bool, err return out, nil } +// UnknownParams implements the ParamValidating interface. +func (vind *cfcCommon) UnknownParams() []string { + return vind.unknownParams +} + // Verify returns true if ids maps to ksids. func (vind *CFC) Verify(_ context.Context, _ VCursor, ids []sqltypes.Value, ksids [][]byte) ([]bool, error) { return vind.verify(ids, ksids) @@ -406,5 +427,5 @@ func xxhash64(in []byte) []byte { } func init() { - Register("cfc", NewCFC) + Register("cfc", newCFC) } diff --git a/go/vt/vtgate/vindexes/cfc_test.go b/go/vt/vtgate/vindexes/cfc_test.go index 2e4ff7e6d00..553d36de6c6 100644 --- a/go/vt/vtgate/vindexes/cfc_test.go +++ b/go/vt/vtgate/vindexes/cfc_test.go @@ -30,94 +30,119 @@ import ( "vitess.io/vitess/go/vt/vterrors" ) -func assertEqualVtError(t *testing.T, expected, actual error) { - // vterrors.Errorf returns a struct containing a stacktrace, which fails - // assert.EqualError since the stacktrace would be guaranteed to be different. - // so just check the error message - if expected == nil { - assert.NoError(t, actual) - } else { - assert.EqualError(t, actual, expected.Error()) +func cfcCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "cfc", + vindexName: "cfc", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "cfc", + expectUnknownParams: expectUnknownParams, } } -func TestCFCBuildCFC(t *testing.T) { - cases := []struct { - testName string - params map[string]string - err error - offsets []int - }{ - { - testName: "no params", - }, - { - testName: "no hash", - params: map[string]string{}, - }, - { - testName: "no hash", - params: map[string]string{"offsets": "[1,2]"}, - }, - { - testName: "no offsets", - params: map[string]string{"hash": "md5"}, - err: vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "CFC vindex requires offsets when hash is defined"), - }, - { - testName: "invalid offset", - params: map[string]string{"hash": "md5", "offsets": "10,12"}, - err: vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets 10,12 to CFC vindex cfc. expected sorted positive ints in brackets"), - }, - { - testName: "invalid offset", - params: map[string]string{"hash": "md5", "offsets": "xxx"}, - err: vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets xxx to CFC vindex cfc. expected sorted positive ints in brackets"), - }, - { - testName: "empty offsets", - params: map[string]string{"hash": "md5", "offsets": "[]"}, - err: vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [] to CFC vindex cfc. expected sorted positive ints in brackets"), - }, - { - testName: "unsorted offsets", - params: map[string]string{"hash": "md5", "offsets": "[10,3]"}, - err: vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [10,3] to CFC vindex cfc. expected sorted positive ints in brackets"), - }, - { - testName: "negative offsets", - params: map[string]string{"hash": "md5", "offsets": "[-1,3]"}, - err: vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [-1,3] to CFC vindex cfc. expected sorted positive ints in brackets"), - }, - { - testName: "normal", - params: map[string]string{"hash": "md5", "offsets": "[3, 7]"}, - offsets: []int{3, 7}, - }, - { - testName: "duplicated offsets", - params: map[string]string{"hash": "md5", "offsets": "[4,4,6]"}, - err: vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [4,4,6] to CFC vindex cfc. expected sorted positive ints in brackets"), - }, +func TestCFCCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + cfcCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + cfcCreateVindexTestCase( + "no hash", + map[string]string{}, + nil, + nil, + ), + cfcCreateVindexTestCase( + "no hash with offsets", + map[string]string{"offsets": "[1,2]"}, + nil, + nil, + ), + cfcCreateVindexTestCase( + "hash with no offsets", + map[string]string{"hash": "md5"}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "CFC vindex requires offsets when hash is defined"), + nil, + ), + cfcCreateVindexTestCase( + "invalid offsets 10,12", + map[string]string{"hash": "md5", "offsets": "10,12"}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets 10,12 to CFC vindex cfc. expected sorted positive ints in brackets"), + nil, + ), + cfcCreateVindexTestCase( + "invalid offsets xxx", + map[string]string{"hash": "md5", "offsets": "xxx"}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets xxx to CFC vindex cfc. expected sorted positive ints in brackets"), + nil, + ), + cfcCreateVindexTestCase( + "empty offsets", + map[string]string{"hash": "md5", "offsets": "[]"}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [] to CFC vindex cfc. expected sorted positive ints in brackets"), + nil, + ), + cfcCreateVindexTestCase( + "unsorted offsets", + map[string]string{"hash": "md5", "offsets": "[10,3]"}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [10,3] to CFC vindex cfc. expected sorted positive ints in brackets"), + nil, + ), + cfcCreateVindexTestCase( + "negative offsets", + map[string]string{"hash": "md5", "offsets": "[-1,3]"}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [-1,3] to CFC vindex cfc. expected sorted positive ints in brackets"), + nil, + ), + cfcCreateVindexTestCase( + "duplicated offsets", + map[string]string{"hash": "md5", "offsets": "[4,4,6]"}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "invalid offsets [4,4,6] to CFC vindex cfc. expected sorted positive ints in brackets"), + nil, + ), + cfcCreateVindexTestCase( + "unknown params", + map[string]string{"hash": "md5", "offsets": "[3, 7]", "hello": "world"}, + nil, + []string{"hello"}, + ), } - for _, tc := range cases { - t.Run(tc.testName, func(t *testing.T) { - cfc, err := NewCFC("cfc", tc.params) - assertEqualVtError(t, tc.err, err) - if cfc != nil { - assert.EqualValues(t, tc.offsets, cfc.(*CFC).offsets) - assert.Equal(t, "cfc", cfc.String()) - assert.Equal(t, 1, cfc.Cost()) - assert.Equal(t, true, cfc.IsUnique()) - assert.Equal(t, false, cfc.NeedsVCursor()) - } - }) - } + testCreateVindexes(t, cases) +} + +func TestCFCCreateVindexOptions(t *testing.T) { + vdx, err := CreateVindex( + "cfc", + "normal", + map[string]string{ + "hash": "md5", + "offsets": "[3, 7]", + }, + ) + require.NotNil(t, vdx) + require.Nil(t, err) + unknownParams := vdx.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + require.EqualValues(t, vdx.(*CFC).offsets, []int{3, 7}) } func makeCFC(t *testing.T, params map[string]string) *CFC { - vind, err := NewCFC("cfc", params) + vind, err := newCFC("cfc", params) require.NoError(t, err) cfc, ok := vind.(*CFC) require.True(t, ok) @@ -225,7 +250,6 @@ func TestCFCComputeKsid(t *testing.T) { } }) } - } func TestCFCComputeKsidXxhash(t *testing.T) { @@ -406,7 +430,6 @@ func TestCFCPrefixMap(t *testing.T) { assert.EqualValues(t, tc.dest, dests[0]) }) } - } func TestCFCPrefixQueryMapNoHash(t *testing.T) { @@ -512,7 +535,6 @@ func TestCFCFindPrefixEscape(t *testing.T) { for _, tc := range cases { assert.EqualValues(t, tc.prefix, string(findPrefix([]byte(tc.str)))) } - } func TestDestinationKeyRangeFromPrefix(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/consistent_lookup.go b/go/vt/vtgate/vindexes/consistent_lookup.go index 3c2166c0aaf..553742b13cc 100644 --- a/go/vt/vtgate/vindexes/consistent_lookup.go +++ b/go/vt/vtgate/vindexes/consistent_lookup.go @@ -35,40 +35,55 @@ import ( "vitess.io/vitess/go/vt/sqlparser" ) +const ( + consistentLookupParamWriteOnly = "write_only" +) + var ( - _ SingleColumn = (*ConsistentLookupUnique)(nil) - _ Lookup = (*ConsistentLookupUnique)(nil) - _ WantOwnerInfo = (*ConsistentLookupUnique)(nil) - _ LookupPlanable = (*ConsistentLookupUnique)(nil) - _ SingleColumn = (*ConsistentLookup)(nil) - _ Lookup = (*ConsistentLookup)(nil) - _ WantOwnerInfo = (*ConsistentLookup)(nil) - _ LookupPlanable = (*ConsistentLookup)(nil) + _ SingleColumn = (*ConsistentLookupUnique)(nil) + _ Lookup = (*ConsistentLookupUnique)(nil) + _ WantOwnerInfo = (*ConsistentLookupUnique)(nil) + _ LookupPlanable = (*ConsistentLookupUnique)(nil) + _ ParamValidating = (*ConsistentLookupUnique)(nil) + _ SingleColumn = (*ConsistentLookup)(nil) + _ Lookup = (*ConsistentLookup)(nil) + _ WantOwnerInfo = (*ConsistentLookup)(nil) + _ LookupPlanable = (*ConsistentLookup)(nil) + _ ParamValidating = (*ConsistentLookup)(nil) + + consistentLookupParams = append( + append(make([]string, 0), lookupInternalParams...), + consistentLookupParamWriteOnly, + ) ) func init() { - Register("consistent_lookup", NewConsistentLookup) - Register("consistent_lookup_unique", NewConsistentLookupUnique) + Register("consistent_lookup", newConsistentLookup) + Register("consistent_lookup_unique", newConsistentLookupUnique) } // ConsistentLookup is a non-unique lookup vindex that can stay // consistent with respect to its owner table. type ConsistentLookup struct { *clCommon + unknownParams []string } -// NewConsistentLookup creates a ConsistentLookup vindex. +// newConsistentLookup creates a ConsistentLookup vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. // from: list of columns in the table that have the 'from' values of the lookup vindex. // to: The 'to' column name of the table. -func NewConsistentLookup(name string, m map[string]string) (Vindex, error) { +func newConsistentLookup(name string, m map[string]string) (Vindex, error) { clc, err := newCLCommon(name, m) if err != nil { return nil, err } - return &ConsistentLookup{clCommon: clc}, nil + return &ConsistentLookup{ + clCommon: clc, + unknownParams: FindUnknownParams(m, consistentLookupParams), + }, nil } // Cost returns the cost of this vindex as 20. @@ -152,6 +167,11 @@ func (lu *ConsistentLookup) AutoCommitEnabled() bool { return lu.lkp.Autocommit } +// UnknownParams implements the ParamValidating interface. +func (lu *ConsistentLookup) UnknownParams() []string { + return lu.unknownParams +} + //==================================================================== // ConsistentLookupUnique defines a vindex that uses a lookup table. @@ -159,20 +179,24 @@ func (lu *ConsistentLookup) AutoCommitEnabled() bool { // Unique and a Lookup. type ConsistentLookupUnique struct { *clCommon + unknownParams []string } -// NewConsistentLookupUnique creates a ConsistentLookupUnique vindex. +// newConsistentLookupUnique creates a ConsistentLookupUnique vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. // from: list of columns in the table that have the 'from' values of the lookup vindex. // to: The 'to' column name of the table. -func NewConsistentLookupUnique(name string, m map[string]string) (Vindex, error) { +func newConsistentLookupUnique(name string, m map[string]string) (Vindex, error) { clc, err := newCLCommon(name, m) if err != nil { return nil, err } - return &ConsistentLookupUnique{clCommon: clc}, nil + return &ConsistentLookupUnique{ + clCommon: clc, + unknownParams: FindUnknownParams(m, consistentLookupParams), + }, nil } // Cost returns the cost of this vindex as 10. @@ -271,7 +295,7 @@ type clCommon struct { func newCLCommon(name string, m map[string]string) (*clCommon, error) { lu := &clCommon{name: name} var err error - lu.writeOnly, err = boolFromMap(m, "write_only") + lu.writeOnly, err = boolFromMap(m, consistentLookupParamWriteOnly) if err != nil { return nil, err } @@ -470,3 +494,8 @@ func (lu *clCommon) GetCommitOrder() vtgatepb.CommitOrder { func (lu *ConsistentLookupUnique) IsBackfilling() bool { return lu.writeOnly } + +// UnknownParams implements the ParamValidating interface. +func (lu *ConsistentLookupUnique) UnknownParams() []string { + return lu.unknownParams +} diff --git a/go/vt/vtgate/vindexes/consistent_lookup_test.go b/go/vt/vtgate/vindexes/consistent_lookup_test.go index 7510a701992..17adbcf748f 100644 --- a/go/vt/vtgate/vindexes/consistent_lookup_test.go +++ b/go/vt/vtgate/vindexes/consistent_lookup_test.go @@ -40,6 +40,60 @@ import ( "vitess.io/vitess/go/vt/vterrors" ) +func consistentLookupCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "consistent_lookup", + vindexName: "consistent_lookup", + vindexParams: vindexParams, + + expectCost: 20, + expectErr: expectErr, + expectIsUnique: false, + expectNeedsVCursor: true, + expectString: "consistent_lookup", + expectUnknownParams: expectUnknownParams, + } +} + +func consistentLookupUniqueCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "consistent_lookup_unique", + vindexName: "consistent_lookup_unique", + vindexParams: vindexParams, + + expectCost: 10, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: true, + expectString: "consistent_lookup_unique", + expectUnknownParams: expectUnknownParams, + } +} + +func TestConsistentLookupCreateVindex(t *testing.T) { + testCaseFs := []func(string, map[string]string, error, []string) createVindexTestCase{ + consistentLookupCreateVindexTestCase, + consistentLookupUniqueCreateVindexTestCase, + } + for _, testCaseF := range testCaseFs { + testLookupCreateVindexInternalCases(t, testCaseF) + } +} + func TestConsistentLookupInit(t *testing.T) { lookup := createConsistentLookup(t, "consistent_lookup", true) cols := []sqlparser.IdentifierCI{ @@ -55,22 +109,6 @@ func TestConsistentLookupInit(t *testing.T) { } } -func TestConsistentLookupInfo(t *testing.T) { - lookup := createConsistentLookup(t, "consistent_lookup", false) - assert.Equal(t, 20, lookup.Cost()) - assert.Equal(t, "consistent_lookup", lookup.String()) - assert.False(t, lookup.IsUnique()) - assert.True(t, lookup.NeedsVCursor()) -} - -func TestConsistentLookupUniqueInfo(t *testing.T) { - lookup := createConsistentLookup(t, "consistent_lookup_unique", false) - assert.Equal(t, 10, lookup.Cost()) - assert.Equal(t, "consistent_lookup_unique", lookup.String()) - assert.True(t, lookup.IsUnique()) - assert.True(t, lookup.NeedsVCursor()) -} - func TestConsistentLookupMap(t *testing.T) { lookup := createConsistentLookup(t, "consistent_lookup", false) vc := &loggingVCursor{} @@ -458,6 +496,7 @@ func createConsistentLookup(t *testing.T, name string, writeOnly bool) SingleCol if err != nil { t.Fatal(err) } + require.Empty(t, l.(ParamValidating).UnknownParams()) cols := []sqlparser.IdentifierCI{ sqlparser.NewIdentifierCI("fc1"), sqlparser.NewIdentifierCI("fc2"), diff --git a/go/vt/vtgate/vindexes/hash.go b/go/vt/vtgate/vindexes/hash.go index a488809aeb8..e523c873fe8 100644 --- a/go/vt/vtgate/vindexes/hash.go +++ b/go/vt/vtgate/vindexes/hash.go @@ -33,9 +33,10 @@ import ( ) var ( - _ SingleColumn = (*Hash)(nil) - _ Reversible = (*Hash)(nil) - _ Hashing = (*Hash)(nil) + _ SingleColumn = (*Hash)(nil) + _ Reversible = (*Hash)(nil) + _ Hashing = (*Hash)(nil) + _ ParamValidating = (*Hash)(nil) ) // Hash defines vindex that hashes an int64 to a KeyspaceId @@ -44,12 +45,16 @@ var ( // Note that at once stage we used a 3DES-based hash here, // but for a null key as in our case, they are completely equivalent. type Hash struct { - name string + name string + unknownParams []string } -// NewHash creates a new Hash. -func NewHash(name string, _ map[string]string) (Vindex, error) { - return &Hash{name: name}, nil +// newHash creates a new Hash. +func newHash(name string, params map[string]string) (Vindex, error) { + return &Hash{ + name: name, + unknownParams: FindUnknownParams(params, nil), + }, nil } // String returns the name of the vindex. @@ -132,6 +137,11 @@ func (vind *Hash) Hash(id sqltypes.Value) ([]byte, error) { return vhash(num), nil } +// UnknownParams implements the ParamValidating interface. +func (vind *Hash) UnknownParams() []string { + return vind.unknownParams +} + var blockDES cipher.Block func init() { @@ -140,7 +150,7 @@ func init() { if err != nil { panic(err) } - Register("hash", NewHash) + Register("hash", newHash) } func vhash(shardKey uint64) []byte { diff --git a/go/vt/vtgate/vindexes/hash_test.go b/go/vt/vtgate/vindexes/hash_test.go index 3a7d1125c3e..04749092358 100644 --- a/go/vt/vtgate/vindexes/hash_test.go +++ b/go/vt/vtgate/vindexes/hash_test.go @@ -21,7 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -31,18 +30,62 @@ import ( var hash SingleColumn func init() { - hv, err := CreateVindex("hash", "nn", map[string]string{"Table": "t", "Column": "c"}) + hv, err := CreateVindex("hash", "nn", map[string]string{}) + unknownParams := hv.(ParamValidating).UnknownParams() + if len(unknownParams) > 0 { + panic("hash test init: expected 0 unknown params") + } if err != nil { panic(err) } hash = hv.(SingleColumn) } -func TestHashInfo(t *testing.T) { - assert.Equal(t, 1, hash.Cost()) - assert.Equal(t, "nn", hash.String()) - assert.True(t, hash.IsUnique()) - assert.False(t, hash.NeedsVCursor()) +func hashCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "hash", + vindexName: "hash", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "hash", + expectUnknownParams: expectUnknownParams, + } +} + +func TestHashCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + hashCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + hashCreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + hashCreateVindexTestCase( + "unknown params", + map[string]string{"hello": "world"}, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestHashMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/lookup.go b/go/vt/vtgate/vindexes/lookup.go index 9ac514175df..b3e14fa01f6 100644 --- a/go/vt/vtgate/vindexes/lookup.go +++ b/go/vt/vtgate/vindexes/lookup.go @@ -27,27 +27,41 @@ import ( vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" ) +const ( + lookupParamNoVerify = "no_verify" + lookupParamWriteOnly = "write_only" +) + var ( - _ SingleColumn = (*LookupUnique)(nil) - _ Lookup = (*LookupUnique)(nil) - _ LookupPlanable = (*LookupUnique)(nil) - _ SingleColumn = (*LookupNonUnique)(nil) - _ Lookup = (*LookupNonUnique)(nil) - _ LookupPlanable = (*LookupNonUnique)(nil) + _ SingleColumn = (*LookupUnique)(nil) + _ Lookup = (*LookupUnique)(nil) + _ LookupPlanable = (*LookupUnique)(nil) + _ ParamValidating = (*LookupUnique)(nil) + _ SingleColumn = (*LookupNonUnique)(nil) + _ Lookup = (*LookupNonUnique)(nil) + _ LookupPlanable = (*LookupNonUnique)(nil) + _ ParamValidating = (*LookupNonUnique)(nil) + + lookupParams = append( + append(make([]string, 0), lookupCommonParams...), + lookupParamNoVerify, + lookupParamWriteOnly, + ) ) func init() { - Register("lookup", NewLookup) - Register("lookup_unique", NewLookupUnique) + Register("lookup", newLookup) + Register("lookup_unique", newLookupUnique) } // LookupNonUnique defines a vindex that uses a lookup table and create a mapping between from ids and KeyspaceId. // It's NonUnique and a Lookup. type LookupNonUnique struct { - name string - writeOnly bool - noVerify bool - lkp lookupInternal + name string + writeOnly bool + noVerify bool + lkp lookupInternal + unknownParams []string } func (ln *LookupNonUnique) GetCommitOrder() vtgatepb.CommitOrder { @@ -172,7 +186,12 @@ func (ln *LookupNonUnique) Query() (selQuery string, arguments []string) { return ln.lkp.query() } -// NewLookup creates a LookupNonUnique vindex. +// UnknownParams implements the ParamValidating interface. +func (ln *LookupNonUnique) UnknownParams() []string { + return ln.unknownParams +} + +// newLookup creates a LookupNonUnique vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. @@ -184,19 +203,22 @@ func (ln *LookupNonUnique) Query() (selQuery string, arguments []string) { // autocommit: setting this to "true" will cause inserts to upsert and deletes to be ignored. // write_only: in this mode, Map functions return the full keyrange causing a full scatter. // no_verify: in this mode, Verify will always succeed. -func NewLookup(name string, m map[string]string) (Vindex, error) { - lookup := &LookupNonUnique{name: name} +func newLookup(name string, m map[string]string) (Vindex, error) { + lookup := &LookupNonUnique{ + name: name, + unknownParams: FindUnknownParams(m, lookupParams), + } cc, err := parseCommonConfig(m) if err != nil { return nil, err } - lookup.writeOnly, err = boolFromMap(m, "write_only") + lookup.writeOnly, err = boolFromMap(m, lookupParamWriteOnly) if err != nil { return nil, err } - lookup.noVerify, err = boolFromMap(m, "no_verify") + lookup.noVerify, err = boolFromMap(m, lookupParamNoVerify) if err != nil { return nil, err } @@ -223,10 +245,11 @@ func ksidsToValues(ksids [][]byte) []sqltypes.Value { // The table is expected to define the id column as unique. It's // Unique and a Lookup. type LookupUnique struct { - name string - writeOnly bool - noVerify bool - lkp lookupInternal + name string + writeOnly bool + noVerify bool + lkp lookupInternal + unknownParams []string } func (lu *LookupUnique) GetCommitOrder() vtgatepb.CommitOrder { @@ -241,7 +264,7 @@ func (lu *LookupUnique) AutoCommitEnabled() bool { return lu.lkp.Autocommit } -// NewLookupUnique creates a LookupUnique vindex. +// newLookupUnique creates a LookupUnique vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. @@ -252,19 +275,22 @@ func (lu *LookupUnique) AutoCommitEnabled() bool { // // autocommit: setting this to "true" will cause deletes to be ignored. // write_only: in this mode, Map functions return the full keyrange causing a full scatter. -func NewLookupUnique(name string, m map[string]string) (Vindex, error) { - lu := &LookupUnique{name: name} +func newLookupUnique(name string, m map[string]string) (Vindex, error) { + lu := &LookupUnique{ + name: name, + unknownParams: FindUnknownParams(m, lookupParams), + } cc, err := parseCommonConfig(m) if err != nil { return nil, err } - lu.writeOnly, err = boolFromMap(m, "write_only") + lu.writeOnly, err = boolFromMap(m, lookupParamWriteOnly) if err != nil { return nil, err } - lu.noVerify, err = boolFromMap(m, "no_verify") + lu.noVerify, err = boolFromMap(m, lookupParamNoVerify) if err != nil { return nil, err } @@ -375,3 +401,8 @@ func (lu *LookupUnique) LookupQuery() (string, error) { func (lu *LookupUnique) Query() (string, []string) { return lu.lkp.query() } + +// UnknownParams implements the ParamValidating interface. +func (ln *LookupUnique) UnknownParams() []string { + return ln.unknownParams +} diff --git a/go/vt/vtgate/vindexes/lookup_hash.go b/go/vt/vtgate/vindexes/lookup_hash.go index 993b9655660..a6fedb4c9a0 100644 --- a/go/vt/vtgate/vindexes/lookup_hash.go +++ b/go/vt/vtgate/vindexes/lookup_hash.go @@ -29,18 +29,29 @@ import ( vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" ) +const ( + lookupHashParamWriteOnly = "write_only" +) + var ( - _ SingleColumn = (*LookupHash)(nil) - _ Lookup = (*LookupHash)(nil) - _ LookupPlanable = (*LookupHash)(nil) - _ SingleColumn = (*LookupHashUnique)(nil) - _ Lookup = (*LookupHashUnique)(nil) - _ LookupPlanable = (*LookupHashUnique)(nil) + _ SingleColumn = (*LookupHash)(nil) + _ Lookup = (*LookupHash)(nil) + _ LookupPlanable = (*LookupHash)(nil) + _ ParamValidating = (*LookupHash)(nil) + _ SingleColumn = (*LookupHashUnique)(nil) + _ Lookup = (*LookupHashUnique)(nil) + _ LookupPlanable = (*LookupHashUnique)(nil) + _ ParamValidating = (*LookupHashUnique)(nil) + + lookupHashParams = append( + append(make([]string, 0), lookupCommonParams...), + lookupHashParamWriteOnly, + ) ) func init() { - Register("lookup_hash", NewLookupHash) - Register("lookup_hash_unique", NewLookupHashUnique) + Register("lookup_hash", newLookupHash) + Register("lookup_hash_unique", newLookupHashUnique) } //==================================================================== @@ -50,12 +61,13 @@ func init() { // NonUnique and a Lookup. // Warning: This Vindex is being deprecated in favor of Lookup type LookupHash struct { - name string - writeOnly bool - lkp lookupInternal + name string + writeOnly bool + lkp lookupInternal + unknownParams []string } -// NewLookupHash creates a LookupHash vindex. +// newLookupHash creates a LookupHash vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. @@ -66,14 +78,17 @@ type LookupHash struct { // // autocommit: setting this to "true" will cause inserts to upsert and deletes to be ignored. // write_only: in this mode, Map functions return the full keyrange causing a full scatter. -func NewLookupHash(name string, m map[string]string) (Vindex, error) { - lh := &LookupHash{name: name} +func newLookupHash(name string, m map[string]string) (Vindex, error) { + lh := &LookupHash{ + name: name, + unknownParams: FindUnknownParams(m, lookupHashParams), + } cc, err := parseCommonConfig(m) if err != nil { return nil, err } - lh.writeOnly, err = boolFromMap(m, "write_only") + lh.writeOnly, err = boolFromMap(m, lookupHashParamWriteOnly) if err != nil { return nil, err } @@ -229,6 +244,11 @@ func (lh *LookupHash) MarshalJSON() ([]byte, error) { return json.Marshal(lh.lkp) } +// UnknownParams satisifes the ParamValidating interface. +func (lh *LookupHash) UnknownParams() []string { + return lh.unknownParams +} + // unhashList unhashes a list of keyspace ids into []sqltypes.Value. func unhashList(ksids [][]byte) ([]sqltypes.Value, error) { values := make([]sqltypes.Value, 0, len(ksids)) @@ -249,14 +269,15 @@ func unhashList(ksids [][]byte) ([]sqltypes.Value, error) { // Unique and a Lookup. // Warning: This Vindex is being depcreated in favor of LookupUnique type LookupHashUnique struct { - name string - writeOnly bool - lkp lookupInternal + name string + writeOnly bool + lkp lookupInternal + unknownParams []string } var _ LookupPlanable = (*LookupHashUnique)(nil) -// NewLookupHashUnique creates a LookupHashUnique vindex. +// newLookupHashUnique creates a LookupHashUnique vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. @@ -267,14 +288,17 @@ var _ LookupPlanable = (*LookupHashUnique)(nil) // // autocommit: setting this to "true" will cause deletes to be ignored. // write_only: in this mode, Map functions return the full keyrange causing a full scatter. -func NewLookupHashUnique(name string, m map[string]string) (Vindex, error) { - lhu := &LookupHashUnique{name: name} +func newLookupHashUnique(name string, m map[string]string) (Vindex, error) { + lhu := &LookupHashUnique{ + name: name, + unknownParams: FindUnknownParams(m, lookupHashParams), + } cc, err := parseCommonConfig(m) if err != nil { return nil, err } - lhu.writeOnly, err = boolFromMap(m, "write_only") + lhu.writeOnly, err = boolFromMap(m, lookupHashParamWriteOnly) if err != nil { return nil, err } @@ -419,3 +443,8 @@ func (lhu *LookupHashUnique) Query() (selQuery string, arguments []string) { func (lhu *LookupHashUnique) GetCommitOrder() vtgatepb.CommitOrder { return vtgatepb.CommitOrder_NORMAL } + +// UnknownParams implements the ParamValidating interface. +func (lhu *LookupHashUnique) UnknownParams() []string { + return lhu.unknownParams +} diff --git a/go/vt/vtgate/vindexes/lookup_hash_test.go b/go/vt/vtgate/vindexes/lookup_hash_test.go index 7703bd4d344..69bff9f6f34 100644 --- a/go/vt/vtgate/vindexes/lookup_hash_test.go +++ b/go/vt/vtgate/vindexes/lookup_hash_test.go @@ -21,7 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -29,6 +28,32 @@ import ( topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) +func lookupHashCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "lookup_hash", + vindexName: "lookup_hash", + vindexParams: vindexParams, + + expectCost: 20, + expectErr: expectErr, + expectIsUnique: false, + expectNeedsVCursor: true, + expectString: "lookup_hash", + expectUnknownParams: expectUnknownParams, + } +} + +func TestLookupHashCreateVindex(t *testing.T) { + testLookupCreateVindexCommonCases(t, lookupHashCreateVindexTestCase) +} + func TestLookupHashNew(t *testing.T) { l := createLookup(t, "lookup_hash", false /* writeOnly */) if want, got := l.(*LookupHash).writeOnly, false; got != want { @@ -40,7 +65,7 @@ func TestLookupHashNew(t *testing.T) { t.Errorf("Create(lookup, false): %v, want %v", got, want) } - _, err := CreateVindex("lookup_hash", "lookup_hash", map[string]string{ + vdx, err := CreateVindex("lookup_hash", "lookup_hash", map[string]string{ "table": "t", "from": "fromc", "to": "toc", @@ -50,20 +75,10 @@ func TestLookupHashNew(t *testing.T) { if err == nil || err.Error() != want { t.Errorf("Create(bad_scatter): %v, want %s", err, want) } -} - -func TestLookupHashInfo(t *testing.T) { - lookuphash := createLookup(t, "lookup_hash", false /* writeOnly */) - assert.Equal(t, 20, lookuphash.Cost()) - assert.Equal(t, "lookup_hash", lookuphash.String()) - assert.False(t, lookuphash.IsUnique()) - assert.True(t, lookuphash.NeedsVCursor()) - - lookuphashunique := createLookup(t, "lookup_hash_unique", false /* writeOnly */) - assert.Equal(t, 10, lookuphashunique.Cost()) - assert.Equal(t, "lookup_hash_unique", lookuphashunique.String()) - assert.True(t, lookuphashunique.IsUnique()) - assert.True(t, lookuphashunique.NeedsVCursor()) + if err == nil { + unknownParams := vdx.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + } } func TestLookupHashMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/lookup_hash_unique_test.go b/go/vt/vtgate/vindexes/lookup_hash_unique_test.go index 9158f99dc04..67697fb5eac 100644 --- a/go/vt/vtgate/vindexes/lookup_hash_unique_test.go +++ b/go/vt/vtgate/vindexes/lookup_hash_unique_test.go @@ -21,7 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -29,24 +28,54 @@ import ( topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) +func lookupHashUniqueCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "lookup_hash_unique", + vindexName: "lookup_hash_unique", + vindexParams: vindexParams, + + expectCost: 10, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: true, + expectString: "lookup_hash_unique", + expectUnknownParams: expectUnknownParams, + } +} + +func TestLookupHashUniqueCreateVindex(t *testing.T) { + testLookupCreateVindexCommonCases(t, lookupHashUniqueCreateVindexTestCase) +} + func TestLookupHashUniqueNew(t *testing.T) { l := createLookup(t, "lookup_hash_unique", false /* writeOnly */) if want, got := l.(*LookupHashUnique).writeOnly, false; got != want { t.Errorf("Create(lookup, false): %v, want %v", got, want) } - vindex, _ := CreateVindex("lookup_hash_unique", "lookup_hash_unique", map[string]string{ + vindex, err := CreateVindex("lookup_hash_unique", "lookup_hash_unique", map[string]string{ "table": "t", "from": "fromc", "to": "toc", "write_only": "true", }) + unknownParams := vindex.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + require.NoError(t, err) + l = vindex.(SingleColumn) if want, got := l.(*LookupHashUnique).writeOnly, true; got != want { t.Errorf("Create(lookup, false): %v, want %v", got, want) } - _, err := CreateVindex("lookup_hash_unique", "lookup_hash_unique", map[string]string{ + vdx, err := CreateVindex("lookup_hash_unique", "lookup_hash_unique", map[string]string{ "table": "t", "from": "fromc", "to": "toc", @@ -56,14 +85,10 @@ func TestLookupHashUniqueNew(t *testing.T) { if err == nil || err.Error() != want { t.Errorf("Create(bad_scatter): %v, want %s", err, want) } -} - -func TestLookupHashUniqueInfo(t *testing.T) { - lhu := createLookup(t, "lookup_hash_unique", false /* writeOnly */) - assert.Equal(t, 10, lhu.Cost()) - assert.Equal(t, "lookup_hash_unique", lhu.String()) - assert.True(t, lhu.IsUnique()) - assert.True(t, lhu.NeedsVCursor()) + if err == nil { + unknownParams = vdx.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + } } func TestLookupHashUniqueMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/lookup_internal.go b/go/vt/vtgate/vindexes/lookup_internal.go index 2644c261407..673b3fcb64b 100644 --- a/go/vt/vtgate/vindexes/lookup_internal.go +++ b/go/vt/vtgate/vindexes/lookup_internal.go @@ -33,17 +33,47 @@ import ( vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) -var ( +const ( readLockExclusive = "exclusive" readLockShared = "shared" readLockNone = "none" readLockDefault = readLockExclusive + lookupCommonParamAutocommit = "autocommit" + lookupCommonParamMultiShardAutocommit = "multi_shard_autocommit" + + lookupInternalParamTable = "table" + lookupInternalParamFrom = "from" + lookupInternalParamTo = "to" + lookupInternalParamIgnoreNulls = "ignore_nulls" + lookupInternalParamBatchLookup = "batch_lookup" + lookupInternalParamReadLock = "read_lock" +) + +var ( readLockExprs = map[string]string{ readLockExclusive: "for update", readLockShared: "lock in share mode", readLockNone: "", } + + // lookupCommonParams are used only by lookup_* vindexes. + lookupCommonParams = append( + append(make([]string, 0), lookupInternalParams...), + lookupCommonParamAutocommit, + lookupCommonParamMultiShardAutocommit, + ) + + // lookupInternalParams are used by both lookup_* vindexes and the newer + // consistent_lookup_* vindexes. + lookupInternalParams = []string{ + lookupInternalParamTable, + lookupInternalParamFrom, + lookupInternalParamTo, + lookupInternalParamIgnoreNulls, + lookupInternalParamBatchLookup, + lookupInternalParamReadLock, + } ) // lookupInternal implements the functions for the Lookup vindexes. @@ -61,26 +91,26 @@ type lookupInternal struct { } func (lkp *lookupInternal) Init(lookupQueryParams map[string]string, autocommit, upsert, multiShardAutocommit bool) error { - lkp.Table = lookupQueryParams["table"] - lkp.To = lookupQueryParams["to"] + lkp.Table = lookupQueryParams[lookupInternalParamTable] + lkp.To = lookupQueryParams[lookupInternalParamTo] var fromColumns []string - for _, from := range strings.Split(lookupQueryParams["from"], ",") { + for _, from := range strings.Split(lookupQueryParams[lookupInternalParamFrom], ",") { fromColumns = append(fromColumns, strings.TrimSpace(from)) } lkp.FromColumns = fromColumns var err error - lkp.IgnoreNulls, err = boolFromMap(lookupQueryParams, "ignore_nulls") + lkp.IgnoreNulls, err = boolFromMap(lookupQueryParams, lookupInternalParamIgnoreNulls) if err != nil { return err } - lkp.BatchLookup, err = boolFromMap(lookupQueryParams, "batch_lookup") + lkp.BatchLookup, err = boolFromMap(lookupQueryParams, lookupInternalParamBatchLookup) if err != nil { return err } - if readLock, ok := lookupQueryParams["read_lock"]; ok { + if readLock, ok := lookupQueryParams[lookupInternalParamReadLock]; ok { if _, valid := readLockExprs[readLock]; !valid { - return fmt.Errorf("invalid read_lock value: %s", readLock) + return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid %s value: %s", lookupInternalParamReadLock, readLock) } lkp.ReadLock = readLock } @@ -399,10 +429,10 @@ type commonConfig struct { func parseCommonConfig(m map[string]string) (*commonConfig, error) { var c commonConfig var err error - if c.autocommit, err = boolFromMap(m, "autocommit"); err != nil { + if c.autocommit, err = boolFromMap(m, lookupCommonParamAutocommit); err != nil { return nil, err } - if c.multiShardAutocommit, err = boolFromMap(m, "multi_shard_autocommit"); err != nil { + if c.multiShardAutocommit, err = boolFromMap(m, lookupCommonParamMultiShardAutocommit); err != nil { return nil, err } return &c, nil diff --git a/go/vt/vtgate/vindexes/lookup_test.go b/go/vt/vtgate/vindexes/lookup_test.go index df21f07c83d..9b01770218a 100644 --- a/go/vt/vtgate/vindexes/lookup_test.go +++ b/go/vt/vtgate/vindexes/lookup_test.go @@ -34,6 +34,8 @@ import ( querypb "vitess.io/vitess/go/vt/proto/query" topodatapb "vitess.io/vitess/go/vt/proto/topodata" vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" ) // LookupNonUnique tests are more comprehensive than others. @@ -112,6 +114,238 @@ func (vc *vcursor) execute(query string, bindvars map[string]*querypb.BindVariab panic("unexpected") } +func lookupCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "lookup", + vindexName: "lookup", + vindexParams: vindexParams, + + expectCost: 20, + expectErr: expectErr, + expectIsUnique: false, + expectNeedsVCursor: true, + expectString: "lookup", + expectUnknownParams: expectUnknownParams, + } +} + +func lookupUniqueCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "lookup_unique", + vindexName: "lookup_unique", + vindexParams: vindexParams, + + expectCost: 10, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: true, + expectString: "lookup_unique", + expectUnknownParams: expectUnknownParams, + } +} + +func testLookupCreateVindexCommonCases(t *testing.T, testCaseF func(string, map[string]string, error, []string) createVindexTestCase) { + testLookupCreateVindexInternalCases(t, testCaseF) + + cases := []createVindexTestCase{ + testCaseF( + "autocommit true", + map[string]string{"autocommit": "true"}, + nil, + nil, + ), + testCaseF( + "autocommit false", + map[string]string{"autocommit": "false"}, + nil, + nil, + ), + testCaseF( + "autocommit reject not bool", + map[string]string{"autocommit": "hello"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "autocommit value must be 'true' or 'false': 'hello'"), + nil, + ), + testCaseF( + "multi_shard_autocommit true", + map[string]string{"multi_shard_autocommit": "true"}, + nil, + nil, + ), + testCaseF( + "multi_shard_autocommit false", + map[string]string{"multi_shard_autocommit": "false"}, + nil, + nil, + ), + testCaseF( + "multi_shard_autocommit reject not bool", + map[string]string{"multi_shard_autocommit": "hello"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "multi_shard_autocommit value must be 'true' or 'false': 'hello'"), + nil, + ), + } + + testCreateVindexes(t, cases) +} + +func testLookupCreateVindexInternalCases(t *testing.T, testCaseF func(string, map[string]string, error, []string) createVindexTestCase) { + cases := []createVindexTestCase{ + // TODO(maxeng): make table, to, from required params. + testCaseF( + "no params", + nil, + nil, + nil, + ), + testCaseF( + "empty params", + map[string]string{}, + nil, + nil, + ), + testCaseF( + "batch_lookup true", + map[string]string{"batch_lookup": "true"}, + nil, + nil, + ), + testCaseF( + "batch_lookup false", + map[string]string{"batch_lookup": "false"}, + nil, + nil, + ), + testCaseF( + "batch_lookup reject not bool", + map[string]string{"batch_lookup": "hello"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "batch_lookup value must be 'true' or 'false': 'hello'"), + nil, + ), + testCaseF( + "ignore_nulls true", + map[string]string{"ignore_nulls": "true"}, + nil, + nil, + ), + testCaseF( + "ignore_nulls false", + map[string]string{"ignore_nulls": "false"}, + nil, + nil, + ), + testCaseF( + "ignore_nulls reject not bool", + map[string]string{"ignore_nulls": "hello"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "ignore_nulls value must be 'true' or 'false': 'hello'"), + nil, + ), + testCaseF( + "read_lock exclusive", + map[string]string{"read_lock": "exclusive"}, + nil, + nil, + ), + testCaseF( + "read_lock shared", + map[string]string{"read_lock": "shared"}, + nil, + nil, + ), + testCaseF( + "read_lock none", + map[string]string{"read_lock": "none"}, + nil, + nil, + ), + testCaseF( + "read_lock reject unknown values", + map[string]string{"read_lock": "unknown"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "invalid read_lock value: unknown"), + nil, + ), + testCaseF( + "ignore_nulls reject not bool", + map[string]string{"ignore_nulls": "hello"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "ignore_nulls value must be 'true' or 'false': 'hello'"), + nil, + ), + testCaseF( + "write_only true", + map[string]string{"write_only": "true"}, + nil, + nil, + ), + testCaseF( + "write_only false", + map[string]string{"write_only": "false"}, + nil, + nil, + ), + testCaseF( + "write_only reject not bool", + map[string]string{"write_only": "hello"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "write_only value must be 'true' or 'false': 'hello'"), + nil, + ), + testCaseF( + "unknown params", + map[string]string{"hello": "world"}, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) +} + +func TestLookupCreateVindex(t *testing.T) { + testCaseFs := []func(string, map[string]string, error, []string) createVindexTestCase{ + lookupCreateVindexTestCase, + lookupUniqueCreateVindexTestCase, + } + for _, testCaseF := range testCaseFs { + testLookupCreateVindexCommonCases(t, testCaseF) + + cases := []createVindexTestCase{ + testCaseF( + "no_verify true", + map[string]string{"no_verify": "true"}, + nil, + nil, + ), + testCaseF( + "no_verify false", + map[string]string{"no_verify": "false"}, + nil, + nil, + ), + testCaseF( + "no_verify reject not bool", + map[string]string{"no_verify": "hello"}, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "no_verify value must be 'true' or 'false': 'hello'"), + nil, + ), + } + + testCreateVindexes(t, cases) + } +} + func TestLookupNonUniqueNew(t *testing.T) { l := createLookup(t, "lookup", false /* writeOnly */) assert.False(t, l.(*LookupNonUnique).writeOnly, "Create(lookup, false)") @@ -128,25 +362,17 @@ func TestLookupNonUniqueNew(t *testing.T) { require.EqualError(t, err, "write_only value must be 'true' or 'false': 'invalid'") } -func TestLookupNonUniqueInfo(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) - assert.Equal(t, 20, lookupNonUnique.Cost()) - assert.Equal(t, "lookup", lookupNonUnique.String()) - assert.False(t, lookupNonUnique.IsUnique()) - assert.True(t, lookupNonUnique.NeedsVCursor()) -} - func TestLookupNilVCursor(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) - _, err := lookupNonUnique.Map(context.Background(), nil, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) + lnu := createLookup(t, "lookup", false /* writeOnly */) + _, err := lnu.Map(context.Background(), nil, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) require.EqualError(t, err, "cannot perform lookup: no vcursor provided") } func TestLookupNonUniqueMap(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) + lnu := createLookup(t, "lookup", false /* writeOnly */) vc := &vcursor{numRows: 2} - got, err := lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) + got, err := lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) require.NoError(t, err) want := []key.Destination{ key.DestinationKeyspaceIDs([][]byte{ @@ -172,7 +398,7 @@ func TestLookupNonUniqueMap(t *testing.T) { // Test query fail. vc.mustFail = true - _, err = lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}) + _, err = lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}) require.EqualError(t, err, "lookup.Map: execute failed") } @@ -184,10 +410,11 @@ func TestLookupNonUniqueMapAutocommit(t *testing.T) { "autocommit": "true", }) require.NoError(t, err) - lookupNonUnique := vindex.(SingleColumn) + require.Empty(t, vindex.(ParamValidating).UnknownParams()) + lnu := vindex.(SingleColumn) vc := &vcursor{numRows: 2} - got, err := lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) + got, err := lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) require.NoError(t, err) want := []key.Destination{ key.DestinationKeyspaceIDs([][]byte{ @@ -214,10 +441,10 @@ func TestLookupNonUniqueMapAutocommit(t *testing.T) { } func TestLookupNonUniqueMapWriteOnly(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", true) + lnu := createLookup(t, "lookup", true) vc := &vcursor{numRows: 0} - got, err := lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) + got, err := lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) require.NoError(t, err) want := []key.Destination{ key.DestinationKeyRange{ @@ -231,10 +458,10 @@ func TestLookupNonUniqueMapWriteOnly(t *testing.T) { } func TestLookupNonUniqueMapAbsent(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) + lnu := createLookup(t, "lookup", false /* writeOnly */) vc := &vcursor{numRows: 0} - got, err := lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) + got, err := lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}) require.NoError(t, err) want := []key.Destination{ key.DestinationNone{}, @@ -244,10 +471,10 @@ func TestLookupNonUniqueMapAbsent(t *testing.T) { } func TestLookupNonUniqueVerify(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) + lnu := createLookup(t, "lookup", false /* writeOnly */) vc := &vcursor{numRows: 1} - _, err := lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte("test1"), []byte("test2")}) + _, err := lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte("test1"), []byte("test2")}) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -267,15 +494,15 @@ func TestLookupNonUniqueVerify(t *testing.T) { // Test query fail. vc.mustFail = true - _, err = lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}) + _, err = lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}) require.EqualError(t, err, "lookup.Verify: execute failed") vc.mustFail = false // writeOnly true should always yield true. - lookupNonUnique = createLookup(t, "lookup", true) + lnu = createLookup(t, "lookup", true) vc.queries = nil - got, err := lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte(""), []byte("")}) + got, err := lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte(""), []byte("")}) require.NoError(t, err) assert.Empty(t, vc.queries, "lookup verify queries") utils.MustMatch(t, []bool{true, true}, got) @@ -289,10 +516,11 @@ func TestLookupNonUniqueNoVerify(t *testing.T) { "no_verify": "true", }) require.NoError(t, err) - lookupNonUnique := vindex.(SingleColumn) + require.Empty(t, vindex.(ParamValidating).UnknownParams()) + lnu := vindex.(SingleColumn) vc := &vcursor{numRows: 1} - _, err = lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte("test1"), []byte("test2")}) + _, err = lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte("test1"), []byte("test2")}) require.NoError(t, err) var wantqueries []*querypb.BoundQuery @@ -300,7 +528,7 @@ func TestLookupNonUniqueNoVerify(t *testing.T) { // Test query fail. vc.mustFail = true - _, err = lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}) + _, err = lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}) require.NoError(t, err) } @@ -312,6 +540,7 @@ func TestLookupUniqueNoVerify(t *testing.T) { "no_verify": "true", }) require.NoError(t, err) + require.Empty(t, vindex.(ParamValidating).UnknownParams()) lookupUnique := vindex.(SingleColumn) vc := &vcursor{numRows: 1} @@ -335,10 +564,11 @@ func TestLookupNonUniqueVerifyAutocommit(t *testing.T) { "autocommit": "true", }) require.NoError(t, err) - lookupNonUnique := vindex.(SingleColumn) + require.Empty(t, vindex.(ParamValidating).UnknownParams()) + lnu := vindex.(SingleColumn) vc := &vcursor{numRows: 1} - _, err = lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte("test1"), []byte("test2")}) + _, err = lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}, [][]byte{[]byte("test1"), []byte("test2")}) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -360,10 +590,10 @@ func TestLookupNonUniqueVerifyAutocommit(t *testing.T) { } func TestLookupNonUniqueCreate(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) + lnu := createLookup(t, "lookup", false /* writeOnly */) vc := &vcursor{} - err := lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}, {sqltypes.NewInt64(2)}}, [][]byte{[]byte("test1"), []byte("test2")}, false /* ignoreMode */) + err := lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}, {sqltypes.NewInt64(2)}}, [][]byte{[]byte("test1"), []byte("test2")}, false /* ignoreMode */) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -379,19 +609,19 @@ func TestLookupNonUniqueCreate(t *testing.T) { // With ignore. vc.queries = nil - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(2)}, {sqltypes.NewInt64(1)}}, [][]byte{[]byte("test2"), []byte("test1")}, true /* ignoreMode */) + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(2)}, {sqltypes.NewInt64(1)}}, [][]byte{[]byte("test2"), []byte("test1")}, true /* ignoreMode */) require.NoError(t, err) wantqueries[0].Sql = "insert ignore into t(fromc, toc) values(:fromc_0, :toc_0), (:fromc_1, :toc_1)" utils.MustMatch(t, wantqueries, vc.queries) // With ignore_nulls off - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(2)}, {sqltypes.NULL}}, [][]byte{[]byte("test2"), []byte("test1")}, true /* ignoreMode */) + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(2)}, {sqltypes.NULL}}, [][]byte{[]byte("test2"), []byte("test1")}, true /* ignoreMode */) assert.EqualError(t, err, "lookup.Create: input has null values: row: 1, col: 0") // With ignore_nulls on vc.queries = nil - lookupNonUnique.(*LookupNonUnique).lkp.IgnoreNulls = true - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(2)}, {sqltypes.NULL}}, [][]byte{[]byte("test2"), []byte("test1")}, true /* ignoreMode */) + lnu.(*LookupNonUnique).lkp.IgnoreNulls = true + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(2)}, {sqltypes.NULL}}, [][]byte{[]byte("test2"), []byte("test1")}, true /* ignoreMode */) require.NoError(t, err) wantqueries = []*querypb.BoundQuery{{ Sql: "insert ignore into t(fromc, toc) values(:fromc_0, :toc_0)", @@ -404,26 +634,27 @@ func TestLookupNonUniqueCreate(t *testing.T) { // Test query fail. vc.mustFail = true - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false /* ignoreMode */) + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false /* ignoreMode */) assert.EqualError(t, err, "lookup.Create: execute failed") vc.mustFail = false // Test column mismatch. - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false /* ignoreMode */) + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false /* ignoreMode */) assert.EqualError(t, err, "lookup.Create: column vindex count does not match the columns in the lookup: 2 vs [fromc]") } func TestLookupNonUniqueCreateAutocommit(t *testing.T) { - lookupNonUnique, err := CreateVindex("lookup", "lookup", map[string]string{ + lnu, err := CreateVindex("lookup", "lookup", map[string]string{ "table": "t", "from": "from1,from2", "to": "toc", "autocommit": "true", }) require.NoError(t, err) + require.Empty(t, lnu.(ParamValidating).UnknownParams()) vc := &vcursor{} - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ sqltypes.NewInt64(1), sqltypes.NewInt64(2), }, { sqltypes.NewInt64(3), sqltypes.NewInt64(4), @@ -446,10 +677,10 @@ func TestLookupNonUniqueCreateAutocommit(t *testing.T) { } func TestLookupNonUniqueDelete(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) + lnu := createLookup(t, "lookup", false /* writeOnly */) vc := &vcursor{} - err := lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}, {sqltypes.NewInt64(2)}}, []byte("test")) + err := lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}, {sqltypes.NewInt64(2)}}, []byte("test")) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -469,35 +700,37 @@ func TestLookupNonUniqueDelete(t *testing.T) { // Test query fail. vc.mustFail = true - err = lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, []byte("\x16k@\xb4J\xbaK\xd6")) + err = lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, []byte("\x16k@\xb4J\xbaK\xd6")) assert.EqualError(t, err, "lookup.Delete: execute failed") vc.mustFail = false // Test column count fail. - err = lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}}, []byte("\x16k@\xb4J\xbaK\xd6")) + err = lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}}, []byte("\x16k@\xb4J\xbaK\xd6")) assert.EqualError(t, err, "lookup.Delete: column vindex count does not match the columns in the lookup: 2 vs [fromc]") } func TestLookupNonUniqueDeleteAutocommit(t *testing.T) { - lookupNonUnique, _ := CreateVindex("lookup", "lookup", map[string]string{ + lnu, err := CreateVindex("lookup", "lookup", map[string]string{ "table": "t", "from": "fromc", "to": "toc", "autocommit": "true", }) + require.NoError(t, err) + require.Empty(t, lnu.(ParamValidating).UnknownParams()) vc := &vcursor{} - err := lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}, {sqltypes.NewInt64(2)}}, []byte("test")) + err = lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}, {sqltypes.NewInt64(2)}}, []byte("test")) require.NoError(t, err) utils.MustMatch(t, []*querypb.BoundQuery(nil), vc.queries) } func TestLookupNonUniqueUpdate(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup", false /* writeOnly */) + lnu := createLookup(t, "lookup", false /* writeOnly */) vc := &vcursor{} - err := lookupNonUnique.(Lookup).Update(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, []byte("test"), []sqltypes.Value{sqltypes.NewInt64(2)}) + err := lnu.(Lookup).Update(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, []byte("test"), []sqltypes.Value{sqltypes.NewInt64(2)}) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -567,16 +800,17 @@ func TestLookupUniqueMapResult(t *testing.T) { } func TestLookupNonUniqueCreateMultiShardAutocommit(t *testing.T) { - lookupNonUnique, err := CreateVindex("lookup", "lookup", map[string]string{ + lnu, err := CreateVindex("lookup", "lookup", map[string]string{ "table": "t", "from": "from1,from2", "to": "toc", "multi_shard_autocommit": "true", }) require.NoError(t, err) + require.Empty(t, lnu.(ParamValidating).UnknownParams()) vc := &vcursor{} - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ sqltypes.NewInt64(1), sqltypes.NewInt64(2), }, { sqltypes.NewInt64(3), sqltypes.NewInt64(4), @@ -611,5 +845,6 @@ func createLookup(t *testing.T, name string, writeOnly bool) SingleColumn { "write_only": write, }) require.NoError(t, err) + require.Empty(t, l.(ParamValidating).UnknownParams()) return l.(SingleColumn) } diff --git a/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash.go b/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash.go index 725511614ae..5659aed6212 100644 --- a/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash.go +++ b/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash.go @@ -30,16 +30,27 @@ import ( vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" ) +const ( + lookupUnicodeLooseMD5HashParamWriteOnly = "write_only" +) + var ( - _ SingleColumn = (*LookupUnicodeLooseMD5Hash)(nil) - _ Lookup = (*LookupUnicodeLooseMD5Hash)(nil) - _ SingleColumn = (*LookupUnicodeLooseMD5HashUnique)(nil) - _ Lookup = (*LookupUnicodeLooseMD5HashUnique)(nil) + _ SingleColumn = (*LookupUnicodeLooseMD5Hash)(nil) + _ Lookup = (*LookupUnicodeLooseMD5Hash)(nil) + _ ParamValidating = (*LookupUnicodeLooseMD5Hash)(nil) + _ SingleColumn = (*LookupUnicodeLooseMD5HashUnique)(nil) + _ Lookup = (*LookupUnicodeLooseMD5HashUnique)(nil) + _ ParamValidating = (*LookupUnicodeLooseMD5HashUnique)(nil) + + lookupUnicodeLooseMD5HashParams = append( + append(make([]string, 0), lookupCommonParams...), + lookupUnicodeLooseMD5HashParamWriteOnly, + ) ) func init() { - Register("lookup_unicodeloosemd5_hash", NewLookupUnicodeLooseMD5Hash) - Register("lookup_unicodeloosemd5_hash_unique", NewLookupUnicodeLooseMD5HashUnique) + Register("lookup_unicodeloosemd5_hash", newLookupUnicodeLooseMD5Hash) + Register("lookup_unicodeloosemd5_hash_unique", newLookupUnicodeLooseMD5HashUnique) } //==================================================================== @@ -49,12 +60,13 @@ func init() { // NonUnique and a Lookup and stores the from value in a hashed form. // Warning: This Vindex is being depcreated in favor of Lookup type LookupUnicodeLooseMD5Hash struct { - name string - writeOnly bool - lkp lookupInternal + name string + writeOnly bool + lkp lookupInternal + unknownParams []string } -// NewLookupUnicodeLooseMD5Hash creates a LookupUnicodeLooseMD5Hash vindex. +// newLookupUnicodeLooseMD5Hash creates a LookupUnicodeLooseMD5Hash vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. @@ -65,14 +77,17 @@ type LookupUnicodeLooseMD5Hash struct { // // autocommit: setting this to "true" will cause inserts to upsert and deletes to be ignored. // write_only: in this mode, Map functions return the full keyrange causing a full scatter. -func NewLookupUnicodeLooseMD5Hash(name string, m map[string]string) (Vindex, error) { - lh := &LookupUnicodeLooseMD5Hash{name: name} +func newLookupUnicodeLooseMD5Hash(name string, m map[string]string) (Vindex, error) { + lh := &LookupUnicodeLooseMD5Hash{ + name: name, + unknownParams: FindUnknownParams(m, lookupUnicodeLooseMD5HashParams), + } cc, err := parseCommonConfig(m) if err != nil { return nil, err } - lh.writeOnly, err = boolFromMap(m, "write_only") + lh.writeOnly, err = boolFromMap(m, lookupUnicodeLooseMD5HashParamWriteOnly) if err != nil { return nil, err } @@ -223,6 +238,11 @@ func (lh *LookupUnicodeLooseMD5Hash) MarshalJSON() ([]byte, error) { return json.Marshal(lh.lkp) } +// UnknownParams implements the ParamValidating interface. +func (lh *LookupUnicodeLooseMD5Hash) UnknownParams() []string { + return lh.unknownParams +} + //==================================================================== // LookupUnicodeLooseMD5HashUnique defines a vindex that uses a lookup table. @@ -230,12 +250,13 @@ func (lh *LookupUnicodeLooseMD5Hash) MarshalJSON() ([]byte, error) { // Unique and a Lookup and will store the from value in a hashed format. // Warning: This Vindex is being depcreated in favor of LookupUnique type LookupUnicodeLooseMD5HashUnique struct { - name string - writeOnly bool - lkp lookupInternal + name string + writeOnly bool + lkp lookupInternal + unknownParams []string } -// NewLookupUnicodeLooseMD5HashUnique creates a LookupUnicodeLooseMD5HashUnique vindex. +// newLookupUnicodeLooseMD5HashUnique creates a LookupUnicodeLooseMD5HashUnique vindex. // The supplied map has the following required fields: // // table: name of the backing table. It can be qualified by the keyspace. @@ -246,20 +267,23 @@ type LookupUnicodeLooseMD5HashUnique struct { // // autocommit: setting this to "true" will cause deletes to be ignored. // write_only: in this mode, Map functions return the full keyrange causing a full scatter. -func NewLookupUnicodeLooseMD5HashUnique(name string, m map[string]string) (Vindex, error) { - lhu := &LookupUnicodeLooseMD5HashUnique{name: name} +func newLookupUnicodeLooseMD5HashUnique(name string, m map[string]string) (Vindex, error) { + lhu := &LookupUnicodeLooseMD5HashUnique{ + name: name, + unknownParams: FindUnknownParams(m, lookupUnicodeLooseMD5HashParams), + } cc, err := parseCommonConfig(m) if err != nil { return nil, err } - lhu.writeOnly, err = boolFromMap(m, "write_only") + lhu.writeOnly, err = boolFromMap(m, lookupUnicodeLooseMD5HashParamWriteOnly) if err != nil { return nil, err } // Don't allow upserts for unique vindexes. - if err := lhu.lkp.Init(m, cc.autocommit, false, cc.multiShardAutocommit); err != nil { + if err := lhu.lkp.Init(m, cc.autocommit, false /* upsert */, cc.multiShardAutocommit); err != nil { return nil, err } return lhu, nil @@ -399,6 +423,11 @@ func (lhu *LookupUnicodeLooseMD5HashUnique) IsBackfilling() bool { return lhu.writeOnly } +// UnknownParams implements the ParamValidating interface. +func (lhu *LookupUnicodeLooseMD5HashUnique) UnknownParams() []string { + return lhu.unknownParams +} + func unicodeHashValue(value sqltypes.Value) (sqltypes.Value, error) { hash, err := unicodeHash(vMD5Hash, value) if err != nil { diff --git a/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash_test.go b/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash_test.go index 373bc374074..989458ccc13 100644 --- a/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash_test.go +++ b/go/vt/vtgate/vindexes/lookup_unicodeloosemd5_hash_test.go @@ -37,6 +37,32 @@ const ( hashed40 uint64 = 16576388050845489136 ) +func lookupUnicodeLooseMD5HashCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "lookup_unicodeloosemd5_hash", + vindexName: "lookup_unicodeloosemd5_hash", + vindexParams: vindexParams, + + expectCost: 20, + expectErr: expectErr, + expectIsUnique: false, + expectNeedsVCursor: true, + expectString: "lookup_unicodeloosemd5_hash", + expectUnknownParams: expectUnknownParams, + } +} + +func TestLookupUnicodeLooseMD5HashCreateVindex(t *testing.T) { + testLookupCreateVindexCommonCases(t, lookupUnicodeLooseMD5HashCreateVindexTestCase) +} + func TestLookupUnicodeLooseMD5HashMap(t *testing.T) { lookup := createLookup(t, "lookup_unicodeloosemd5_hash", false) vc := &vcursor{numRows: 2, keys: []sqltypes.Value{sqltypes.NewUint64(hashed10), sqltypes.NewUint64(hashed20)}} @@ -84,16 +110,17 @@ func TestLookupUnicodeLooseMD5HashMapAutocommit(t *testing.T) { "table": "t", "from": "fromc", "to": "toc", - "hash_from": "true", "autocommit": "true", }) if err != nil { t.Fatal(err) } - lookupNonUnique := vindex.(SingleColumn) + unknownParams := vindex.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + lnu := vindex.(SingleColumn) vc := &vcursor{numRows: 2, keys: []sqltypes.Value{sqltypes.NewUint64(hashed10), sqltypes.NewUint64(hashed20)}} - got, err := lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}) + got, err := lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}) require.NoError(t, err) want := []key.Destination{ key.DestinationKeyspaceIDs([][]byte{ @@ -127,10 +154,10 @@ func TestLookupUnicodeLooseMD5HashMapAutocommit(t *testing.T) { } func TestLookupUnicodeLooseMD5HashMapWriteOnly(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup_unicodeloosemd5_hash", true) + lnu := createLookup(t, "lookup_unicodeloosemd5_hash", true) vc := &vcursor{numRows: 0} - got, err := lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}) + got, err := lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}) require.NoError(t, err) want := []key.Destination{ key.DestinationKeyRange{ @@ -146,10 +173,10 @@ func TestLookupUnicodeLooseMD5HashMapWriteOnly(t *testing.T) { } func TestLookupUnicodeLooseMD5HashMapAbsent(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup_unicodeloosemd5_hash", false) + lnu := createLookup(t, "lookup_unicodeloosemd5_hash", false) vc := &vcursor{numRows: 0} - got, err := lookupNonUnique.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}) + got, err := lnu.Map(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}) require.NoError(t, err) want := []key.Destination{ key.DestinationNone{}, @@ -161,10 +188,10 @@ func TestLookupUnicodeLooseMD5HashMapAbsent(t *testing.T) { } func TestLookupUnicodeLooseMD5HashVerify(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup_unicodeloosemd5_hash", false) + lnu := createLookup(t, "lookup_unicodeloosemd5_hash", false) vc := &vcursor{numRows: 1} - got, err := lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}) + got, err := lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}) require.NoError(t, err) wantResult := []bool{true, true} if !reflect.DeepEqual(got, wantResult) { @@ -190,7 +217,7 @@ func TestLookupUnicodeLooseMD5HashVerify(t *testing.T) { // Test query fail. vc.mustFail = true - _, err = lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}) + _, err = lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(1)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}) want := "lookup.Verify: execute failed" if err == nil || err.Error() != want { t.Errorf("lookupNonUnique(query fail) err: %v, want %s", err, want) @@ -198,10 +225,10 @@ func TestLookupUnicodeLooseMD5HashVerify(t *testing.T) { vc.mustFail = false // writeOnly true should always yield true. - lookupNonUnique = createLookup(t, "lookup_unicodeloosemd5_hash", true) + lnu = createLookup(t, "lookup_unicodeloosemd5_hash", true) vc.queries = nil - got, err = lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}, [][]byte{[]byte(""), []byte("")}) + got, err = lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}, [][]byte{[]byte(""), []byte("")}) require.NoError(t, err) if vc.queries != nil { t.Errorf("lookup.Verify(writeOnly), queries: %v, want nil", vc.queries) @@ -222,10 +249,12 @@ func TestLookupUnicodeLooseMD5HashVerifyAutocommit(t *testing.T) { if err != nil { t.Fatal(err) } - lookupNonUnique := vindex.(SingleColumn) + unknownParams := vindex.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + lnu := vindex.(SingleColumn) vc := &vcursor{numRows: 1} - _, err = lookupNonUnique.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}) + _, err = lnu.Verify(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -251,10 +280,10 @@ func TestLookupUnicodeLooseMD5HashVerifyAutocommit(t *testing.T) { } func TestLookupUnicodeLooseMD5HashCreate(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup_unicodeloosemd5_hash", false) + lnu := createLookup(t, "lookup_unicodeloosemd5_hash", false) vc := &vcursor{} - err := lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}, false) + err := lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}, false) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -272,7 +301,7 @@ func TestLookupUnicodeLooseMD5HashCreate(t *testing.T) { // With ignore. vc.queries = nil - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}, true) + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6"), []byte("\x06\xe7\xea\"Î’p\x8f")}, true) require.NoError(t, err) wantqueries[0].Sql = "insert ignore into t(fromc, toc) values(:fromc_0, :toc_0), (:fromc_1, :toc_1)" @@ -282,7 +311,7 @@ func TestLookupUnicodeLooseMD5HashCreate(t *testing.T) { // Test query fail. vc.mustFail = true - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false) + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false) want := "lookup.Create: execute failed" if err == nil || err.Error() != want { t.Errorf("lookupNonUnique(query fail) err: %v, want %s", err, want) @@ -290,7 +319,7 @@ func TestLookupUnicodeLooseMD5HashCreate(t *testing.T) { vc.mustFail = false // Test column mismatch. - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false) + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10), sqltypes.NewInt64(20)}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false) want = "lookup.Create: column vindex count does not match the columns in the lookup: 2 vs [fromc]" if err == nil || err.Error() != want { t.Errorf("lookupNonUnique(query fail) err: %v, want %s", err, want) @@ -298,7 +327,7 @@ func TestLookupUnicodeLooseMD5HashCreate(t *testing.T) { } func TestLookupUnicodeLooseMD5HashCreateAutocommit(t *testing.T) { - lookupNonUnique, err := CreateVindex("lookup_unicodeloosemd5_hash", "lookup", map[string]string{ + lnu, err := CreateVindex("lookup_unicodeloosemd5_hash", "lookup", map[string]string{ "table": "t", "from": "from1,from2", "to": "toc", @@ -307,9 +336,11 @@ func TestLookupUnicodeLooseMD5HashCreateAutocommit(t *testing.T) { if err != nil { t.Fatal(err) } + unknownParams := lnu.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) vc := &vcursor{} - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ sqltypes.NewInt64(10), sqltypes.NewInt64(20), }, { sqltypes.NewInt64(30), sqltypes.NewInt64(40), @@ -337,7 +368,7 @@ func TestLookupUnicodeLooseMD5HashCreateAutocommit(t *testing.T) { } func TestLookupUnicodeLooseMD5HashCreateMultiShardAutocommit(t *testing.T) { - lookupNonUnique, err := CreateVindex("lookup_unicodeloosemd5_hash", "lookup", map[string]string{ + lnu, err := CreateVindex("lookup_unicodeloosemd5_hash", "lookup", map[string]string{ "table": "t", "from": "from1,from2", "to": "toc", @@ -346,9 +377,11 @@ func TestLookupUnicodeLooseMD5HashCreateMultiShardAutocommit(t *testing.T) { if err != nil { t.Fatal(err) } + unknownParams := lnu.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) vc := &vcursor{} - err = lookupNonUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ + err = lnu.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{ sqltypes.NewInt64(10), sqltypes.NewInt64(20), }, { sqltypes.NewInt64(30), sqltypes.NewInt64(40), @@ -376,10 +409,10 @@ func TestLookupUnicodeLooseMD5HashCreateMultiShardAutocommit(t *testing.T) { } func TestLookupUnicodeLooseMD5HashDelete(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup_unicodeloosemd5_hash", false) + lnu := createLookup(t, "lookup_unicodeloosemd5_hash", false) vc := &vcursor{} - err := lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, []byte("\x16k@\xb4J\xbaK\xd6")) + err := lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, []byte("\x16k@\xb4J\xbaK\xd6")) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ @@ -401,7 +434,7 @@ func TestLookupUnicodeLooseMD5HashDelete(t *testing.T) { // Test query fail. vc.mustFail = true - err = lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, []byte("\x16k@\xb4J\xbaK\xd6")) + err = lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, []byte("\x16k@\xb4J\xbaK\xd6")) want := "lookup.Delete: execute failed" if err == nil || err.Error() != want { t.Errorf("lookupNonUnique(query fail) err: %v, want %s", err, want) @@ -409,7 +442,7 @@ func TestLookupUnicodeLooseMD5HashDelete(t *testing.T) { vc.mustFail = false // Test column count fail. - err = lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}}, []byte("\x16k@\xb4J\xbaK\xd6")) + err = lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1), sqltypes.NewInt64(2)}}, []byte("\x16k@\xb4J\xbaK\xd6")) want = "lookup.Delete: column vindex count does not match the columns in the lookup: 2 vs [fromc]" if err == nil || err.Error() != want { t.Errorf("lookupNonUnique(query fail) err: %v, want %s", err, want) @@ -417,15 +450,19 @@ func TestLookupUnicodeLooseMD5HashDelete(t *testing.T) { } func TestLookupUnicodeLooseMD5HashDeleteAutocommit(t *testing.T) { - lookupNonUnique, _ := CreateVindex("lookup_unicodeloosemd5_hash", "lookup", map[string]string{ + lnu, err := CreateVindex("lookup_unicodeloosemd5_hash", "lookup", map[string]string{ "table": "t", "from": "fromc", "to": "toc", "autocommit": "true", }) + unknownParams := lnu.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + require.NoError(t, err) + vc := &vcursor{} - err := lookupNonUnique.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, []byte("\x16k@\xb4J\xbaK\xd6")) + err = lnu.(Lookup).Delete(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(10)}, {sqltypes.NewInt64(20)}}, []byte("\x16k@\xb4J\xbaK\xd6")) require.NoError(t, err) wantqueries := []*querypb.BoundQuery(nil) @@ -435,10 +472,10 @@ func TestLookupUnicodeLooseMD5HashDeleteAutocommit(t *testing.T) { } func TestLookupUnicodeLooseMD5HashUpdate(t *testing.T) { - lookupNonUnique := createLookup(t, "lookup_unicodeloosemd5_hash", false) + lnu := createLookup(t, "lookup_unicodeloosemd5_hash", false) vc := &vcursor{} - err := lookupNonUnique.(Lookup).Update(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10)}, []byte("\x16k@\xb4J\xbaK\xd6"), []sqltypes.Value{sqltypes.NewInt64(20)}) + err := lnu.(Lookup).Update(context.Background(), vc, []sqltypes.Value{sqltypes.NewInt64(10)}, []byte("\x16k@\xb4J\xbaK\xd6"), []sqltypes.Value{sqltypes.NewInt64(20)}) require.NoError(t, err) wantqueries := []*querypb.BoundQuery{{ diff --git a/go/vt/vtgate/vindexes/lookup_unique_test.go b/go/vt/vtgate/vindexes/lookup_unique_test.go index cc04d5340c3..fd2a62c4d21 100644 --- a/go/vt/vtgate/vindexes/lookup_unique_test.go +++ b/go/vt/vtgate/vindexes/lookup_unique_test.go @@ -21,7 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -36,18 +35,21 @@ func TestLookupUniqueNew(t *testing.T) { t.Errorf("Create(lookup, false): %v, want %v", got, want) } - vindex, _ := CreateVindex("lookup_unique", "lookup_unique", map[string]string{ + vindex, err := CreateVindex("lookup_unique", "lookup_unique", map[string]string{ "table": "t", "from": "fromc", "to": "toc", "write_only": "true", }) + require.NoError(t, err) + require.Empty(t, vindex.(ParamValidating).UnknownParams()) + l = vindex.(SingleColumn) if want, got := l.(*LookupUnique).writeOnly, true; got != want { t.Errorf("Create(lookup, false): %v, want %v", got, want) } - _, err := CreateVindex("lookup_unique", "lookup_unique", map[string]string{ + _, err = CreateVindex("lookup_unique", "lookup_unique", map[string]string{ "table": "t", "from": "fromc", "to": "toc", @@ -59,13 +61,6 @@ func TestLookupUniqueNew(t *testing.T) { } } -func TestLookupUniqueInfo(t *testing.T) { - lookupUnique := createLookup(t, "lookup_unique", false) - assert.Equal(t, 10, lookupUnique.Cost()) - assert.Equal(t, "lookup_unique", lookupUnique.String()) - assert.True(t, lookupUnique.IsUnique()) -} - func TestLookupUniqueMap(t *testing.T) { lookupUnique := createLookup(t, "lookup_unique", false) vc := &vcursor{numRows: 1} @@ -163,6 +158,7 @@ func TestLookupUniqueCreate(t *testing.T) { if err != nil { t.Fatal(err) } + require.Empty(t, lookupUnique.(ParamValidating).UnknownParams()) vc := &vcursor{} err = lookupUnique.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, [][]byte{[]byte("test")}, false /* ignoreMode */) diff --git a/go/vt/vtgate/vindexes/main_test.go b/go/vt/vtgate/vindexes/main_test.go new file mode 100644 index 00000000000..226ecfff431 --- /dev/null +++ b/go/vt/vtgate/vindexes/main_test.go @@ -0,0 +1,94 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed 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 vindexes + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type createVindexTestCase struct { + testName string + + vindexType string + vindexName string + vindexParams map[string]string + + expectCost int + expectErr error + expectIsUnique bool + expectNeedsVCursor bool + expectString string + expectUnknownParams []string +} + +func assertEqualVtError(t *testing.T, expected, actual error) { + // vterrors.Errorf returns a struct containing a stacktrace, which fails + // assert.EqualError since the stacktrace would be guaranteed to be different. + // so just check the error message + if expected == nil { + assert.NoError(t, actual) + } else { + assert.EqualError(t, actual, expected.Error()) + } +} + +func testCreateVindex( + t *testing.T, + tc createVindexTestCase, + fns ...func(createVindexTestCase, Vindex, []error, error), +) { + t.Run(tc.testName, func(t *testing.T) { + vdx, err := CreateVindex( + tc.vindexType, + tc.vindexName, + tc.vindexParams, + ) + assertEqualVtError(t, tc.expectErr, err) + if err == nil { + assert.NotNil(t, vdx) + } + paramValidating, ok := vdx.(ParamValidating) + var unknownParams []string + if ok { + unknownParams = paramValidating.UnknownParams() + } + require.Equal(t, len(tc.expectUnknownParams), len(unknownParams)) + sort.Strings(tc.expectUnknownParams) + sort.Strings(unknownParams) + require.Equal(t, tc.expectUnknownParams, unknownParams) + if vdx != nil { + assert.Equal(t, tc.expectString, vdx.String()) + assert.Equal(t, tc.expectCost, vdx.Cost()) + assert.Equal(t, tc.expectIsUnique, vdx.IsUnique()) + assert.Equal(t, tc.expectNeedsVCursor, vdx.NeedsVCursor()) + } + }) +} + +func testCreateVindexes( + t *testing.T, + tcs []createVindexTestCase, + fns ...func(createVindexTestCase, Vindex, []error, error), +) { + for _, tc := range tcs { + testCreateVindex(t, tc, fns...) + } +} diff --git a/go/vt/vtgate/vindexes/multicol.go b/go/vt/vtgate/vindexes/multicol.go index 04d84fadb3a..ee53ea5bb60 100644 --- a/go/vt/vtgate/vindexes/multicol.go +++ b/go/vt/vtgate/vindexes/multicol.go @@ -29,7 +29,9 @@ import ( "vitess.io/vitess/go/vt/vterrors" ) -var _ MultiColumn = (*MultiCol)(nil) +var ( + _ MultiColumn = (*MultiCol)(nil) +) type MultiCol struct { name string @@ -46,8 +48,8 @@ const ( defaultVindex = "hash" ) -// NewMultiCol creates a new MultiCol. -func NewMultiCol(name string, m map[string]string) (Vindex, error) { +// newMultiCol creates a new MultiCol. +func newMultiCol(name string, m map[string]string) (Vindex, error) { colCount, err := getColumnCount(m) if err != nil { return nil, err @@ -150,7 +152,7 @@ func (m *MultiCol) mapKsid(colValues []sqltypes.Value) (bool, []byte, error) { } func init() { - Register("multicol", NewMultiCol) + Register("multicol", newMultiCol) } func getColumnVindex(m map[string]string, colCount int) (map[int]Hashing, int, error) { @@ -164,6 +166,15 @@ func getColumnVindex(m map[string]string, colCount int) (map[int]Hashing, int, e } columnVdx := make(map[int]Hashing, colCount) vindexCost := 0 + subParams := make(map[string]string) + for k, v := range m { + if k == paramColumnCount || + k == paramColumnBytes || + k == paramColumnVindex { + continue + } + subParams[k] = v + } for i := 0; i < colCount; i++ { selVdx := defaultVindex if len(colVdxs) > i { @@ -173,7 +184,7 @@ func getColumnVindex(m map[string]string, colCount int) (map[int]Hashing, int, e } } // TODO: reuse vindex. avoid creating same vindex. - vdx, err := CreateVindex(selVdx, selVdx, m) + vdx, err := CreateVindex(selVdx, selVdx, subParams) if err != nil { return nil, 0, err } diff --git a/go/vt/vtgate/vindexes/multicol_test.go b/go/vt/vtgate/vindexes/multicol_test.go index ce2e57dcb0e..e4e2098dd1b 100644 --- a/go/vt/vtgate/vindexes/multicol_test.go +++ b/go/vt/vtgate/vindexes/multicol_test.go @@ -23,29 +23,199 @@ import ( "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func multicolCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectCost int, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "multicol", + vindexName: "multicol", + vindexParams: vindexParams, + + expectCost: expectCost, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "multicol", + expectUnknownParams: expectUnknownParams, + } +} + +func TestMulticolCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + multicolCreateVindexTestCase( + "column count 0 invalid", + map[string]string{ + "column_count": "0", + }, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "number of columns should be between 1 and 8 in the parameter 'column_count'"), + nil, + ), + multicolCreateVindexTestCase( + "column count 3 ok", + map[string]string{ + "column_count": "3", + }, + 3, + nil, + nil, + ), + multicolCreateVindexTestCase( + "column count 9 invalid", + map[string]string{ + "column_count": "9", + }, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "number of columns should be between 1 and 8 in the parameter 'column_count'"), + nil, + ), + multicolCreateVindexTestCase( + "column bytes ok", + map[string]string{ + "column_count": "3", + "column_bytes": "1,2,3", + }, + 3, + nil, + nil, + ), + multicolCreateVindexTestCase( + "column bytes more than column count invalid", + map[string]string{ + "column_count": "3", + "column_bytes": "1,2,3,4", + }, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "number of column bytes provided are more than column count in the parameter 'column_bytes'"), + nil, + ), + multicolCreateVindexTestCase( + "column bytes exceeds keyspace id length", + map[string]string{ + "column_count": "3", + "column_bytes": "100,200,300", + }, + 3, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "column bytes count exceeds the keyspace id length (total bytes count cannot exceed 8 bytes) in the parameter 'column_bytes'"), + nil, + ), + multicolCreateVindexTestCase( + "column vindex ok", + map[string]string{ + "column_count": "3", + "column_bytes": "1,2,3", + "column_vindex": "binary,binary,binary", + }, + 0, + nil, + nil, + ), + multicolCreateVindexTestCase( + "column vindex more than column count", + map[string]string{ + "column_count": "3", + "column_bytes": "1,2,3", + "column_vindex": "binary,binary,binary,binary", + }, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "number of vindex function provided are more than column count in the parameter 'column_vindex'"), + nil, + ), + multicolCreateVindexTestCase( + "column vindex non-hashing invalid", + map[string]string{ + "column_count": "3", + "column_bytes": "1,2,3", + "column_vindex": "binary,binary,null", + }, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "multicol vindex supports vindexes that exports hashing function, are unique and are non-lookup vindex, passed vindex 'null' is invalid"), + nil, + ), + multicolCreateVindexTestCase( + "column vindex non-unique invalid", + map[string]string{ + "column_count": "3", + "column_bytes": "1,2,3", + "column_vindex": "binary,binary,cfc", + }, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "multicol vindex supports vindexes that exports hashing function, are unique and are non-lookup vindex, passed vindex 'cfc' is invalid"), + nil, + ), + multicolCreateVindexTestCase( + "column vindex lookup or needs vcursor invalid", + map[string]string{ + "column_count": "3", + "column_bytes": "1,2,3", + "column_vindex": "binary,binary,lookup", + }, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "multicol vindex supports vindexes that exports hashing function, are unique and are non-lookup vindex, passed vindex 'lookup' is invalid"), + nil, + ), + multicolCreateVindexTestCase( + "no params", + nil, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "number of columns not provided in the parameter 'column_count'"), + nil, + ), + multicolCreateVindexTestCase( + "empty params", + map[string]string{}, + 0, + vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "number of columns not provided in the parameter 'column_count'"), + nil, + ), + multicolCreateVindexTestCase( + "allow unknown params", + map[string]string{ + "column_count": "1", + "hello": "world", + }, + 1, + nil, + nil, + ), + } + + testCreateVindexes(t, cases) +} + func TestMultiColMisc(t *testing.T) { - vindex, err := CreateVindex("multicol", "multicol", map[string]string{ + vindex, err := CreateVindex("multicol", "multicol_misc", map[string]string{ "column_count": "3", }) require.NoError(t, err) + _, ok := vindex.(ParamValidating) + require.False(t, ok) multiColVdx, isMultiColVdx := vindex.(*MultiCol) assert.True(t, isMultiColVdx) assert.Equal(t, 3, multiColVdx.Cost()) - assert.Equal(t, "multicol", multiColVdx.String()) + assert.Equal(t, "multicol_misc", multiColVdx.String()) assert.True(t, multiColVdx.IsUnique()) assert.False(t, multiColVdx.NeedsVCursor()) assert.True(t, multiColVdx.PartialVindex()) } func TestMultiColMap(t *testing.T) { - vindex, err := CreateVindex("multicol", "multicol", map[string]string{ + vindex, err := CreateVindex("multicol", "multicol_map", map[string]string{ "column_count": "3", }) require.NoError(t, err) diff --git a/go/vt/vtgate/vindexes/null.go b/go/vt/vtgate/vindexes/null.go index 3e8085b7501..58435643ea7 100644 --- a/go/vt/vtgate/vindexes/null.go +++ b/go/vt/vtgate/vindexes/null.go @@ -25,8 +25,10 @@ import ( ) var ( - _ Vindex = (*Null)(nil) - nullksid = []byte{0} + _ Vindex = (*Null)(nil) + _ ParamValidating = (*Null)(nil) + + nullksid = []byte{0} ) // Null defines a vindex that always return 0. It's Unique and @@ -36,12 +38,16 @@ var ( // Unlike other vindexes, this one will work even for NULL input values. This // will allow you to keep MySQL auto-inc columns unchanged. type Null struct { - name string + name string + unknownParams []string } -// NewNull creates a new Null. -func NewNull(name string, m map[string]string) (Vindex, error) { - return &Null{name: name}, nil +// newNull creates a new Null. +func newNull(name string, m map[string]string) (Vindex, error) { + return &Null{ + name: name, + unknownParams: FindUnknownParams(m, nil), + }, nil } // String returns the name of the vindex. @@ -82,6 +88,11 @@ func (vind *Null) Verify(ctx context.Context, vcursor VCursor, ids []sqltypes.Va return out, nil } +// UnknownParams implements the ParamValidating interface. +func (vind *Null) UnknownParams() []string { + return vind.unknownParams +} + func init() { - Register("null", NewNull) + Register("null", newNull) } diff --git a/go/vt/vtgate/vindexes/null_test.go b/go/vt/vtgate/vindexes/null_test.go index 87baea46ee3..03b97fe651b 100644 --- a/go/vt/vtgate/vindexes/null_test.go +++ b/go/vt/vtgate/vindexes/null_test.go @@ -21,7 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -31,18 +30,62 @@ import ( var null SingleColumn func init() { - hv, err := CreateVindex("null", "nn", map[string]string{"Table": "t", "Column": "c"}) + hv, err := CreateVindex("null", "nn", map[string]string{}) if err != nil { panic(err) } + unknownParams := hv.(ParamValidating).UnknownParams() + if len(unknownParams) > 0 { + panic("null test init: expected 0 unknown params") + } null = hv.(SingleColumn) } -func TestNullInfo(t *testing.T) { - assert.Equal(t, 100, null.Cost()) - assert.Equal(t, "nn", null.String()) - assert.True(t, null.IsUnique()) - assert.False(t, null.NeedsVCursor()) +func nullCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "null", + vindexName: "null", + vindexParams: vindexParams, + + expectCost: 100, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "null", + expectUnknownParams: expectUnknownParams, + } +} + +func TestNullCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + nullCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + nullCreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + nullCreateVindexTestCase( + "unknown params", + map[string]string{"hello": "world"}, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestNullMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/numeric.go b/go/vt/vtgate/vindexes/numeric.go index e2f8b512fb9..c99663b72e7 100644 --- a/go/vt/vtgate/vindexes/numeric.go +++ b/go/vt/vtgate/vindexes/numeric.go @@ -29,20 +29,25 @@ import ( ) var ( - _ SingleColumn = (*Numeric)(nil) - _ Reversible = (*Numeric)(nil) - _ Hashing = (*Numeric)(nil) + _ SingleColumn = (*Numeric)(nil) + _ Reversible = (*Numeric)(nil) + _ Hashing = (*Numeric)(nil) + _ ParamValidating = (*Numeric)(nil) ) // Numeric defines a bit-pattern mapping of a uint64 to the KeyspaceId. // It's Unique and Reversible. type Numeric struct { - name string + name string + unknownParams []string } -// NewNumeric creates a Numeric vindex. -func NewNumeric(name string, _ map[string]string) (Vindex, error) { - return &Numeric{name: name}, nil +// newNumeric creates a Numeric vindex. +func newNumeric(name string, m map[string]string) (Vindex, error) { + return &Numeric{ + name: name, + unknownParams: FindUnknownParams(m, nil), + }, nil } // String returns the name of the vindex. @@ -105,6 +110,11 @@ func (*Numeric) ReverseMap(_ VCursor, ksids [][]byte) ([]sqltypes.Value, error) return reverseIds, nil } +// UnknownParams implements the ParamValidating interface. +func (vind *Numeric) UnknownParams() []string { + return vind.unknownParams +} + func (*Numeric) Hash(id sqltypes.Value) ([]byte, error) { num, err := evalengine.ToUint64(id) if err != nil { @@ -116,5 +126,5 @@ func (*Numeric) Hash(id sqltypes.Value) ([]byte, error) { } func init() { - Register("numeric", NewNumeric) + Register("numeric", newNumeric) } diff --git a/go/vt/vtgate/vindexes/numeric_static_map.go b/go/vt/vtgate/vindexes/numeric_static_map.go index 11a64e98ce0..5f6dd1ff1ff 100644 --- a/go/vt/vtgate/vindexes/numeric_static_map.go +++ b/go/vt/vtgate/vindexes/numeric_static_map.go @@ -21,19 +21,33 @@ import ( "context" "encoding/binary" "encoding/json" - "errors" "os" "strconv" + "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" ) +const ( + numericStaticMapParamJSON = "json" + numericStaticMapParamJSONPath = "json_path" + numericStaticMapParamFallbackType = "fallback_type" +) + var ( - _ SingleColumn = (*NumericStaticMap)(nil) - _ Hashing = (*NumericStaticMap)(nil) + _ SingleColumn = (*NumericStaticMap)(nil) + _ Hashing = (*NumericStaticMap)(nil) + _ ParamValidating = (*NumericStaticMap)(nil) + + numericStaticMapParams = []string{ + numericStaticMapParamJSON, + numericStaticMapParamJSONPath, + numericStaticMapParamFallbackType, + } ) // NumericLookupTable stores the mapping of keys. @@ -42,26 +56,27 @@ type NumericLookupTable map[uint64]uint64 // NumericStaticMap is similar to vindex Numeric but first attempts a lookup via // a JSON file. type NumericStaticMap struct { - name string - hashVdx Hashing - lookup NumericLookupTable + name string + hashVdx Hashing + lookup NumericLookupTable + unknownParams []string } func init() { - Register("numeric_static_map", NewNumericStaticMap) + Register("numeric_static_map", newNumericStaticMap) } -// NewNumericStaticMap creates a NumericStaticMap vindex. -func NewNumericStaticMap(name string, params map[string]string) (Vindex, error) { - jsonStr, jsok := params["json"] - jsonPath, jpok := params["json_path"] +// newNumericStaticMap creates a NumericStaticMap vindex. +func newNumericStaticMap(name string, params map[string]string) (Vindex, error) { + jsonStr, jsok := params[numericStaticMapParamJSON] + jsonPath, jpok := params[numericStaticMapParamJSONPath] if !jsok && !jpok { - return nil, errors.New("NumericStaticMap: Could not find either `json_path` params in vschema") + return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "NumericStaticMap: Could not find either `json_path` or `json` params in vschema") } if jsok && jpok { - return nil, errors.New("NumericStaticMap: Found both `json` and `json_path` params in vschema") + return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "NumericStaticMap: Found both `json` and `json_path` params in vschema") } var err error @@ -83,19 +98,19 @@ func NewNumericStaticMap(name string, params map[string]string) (Vindex, error) var hashVdx Hashing - if s, ok := params["fallback_type"]; ok { + if s, ok := params[numericStaticMapParamFallbackType]; ok { vindex, err := CreateVindex(s, name+"_hash", map[string]string{}) if err != nil { return nil, err } hashVdx, _ = vindex.(Hashing) // We know this will not fail - } return &NumericStaticMap{ - hashVdx: hashVdx, - lookup: lt, - name: name, + hashVdx: hashVdx, + lookup: lt, + name: name, + unknownParams: FindUnknownParams(params, numericStaticMapParams), }, nil } @@ -166,6 +181,11 @@ func (vind *NumericStaticMap) Hash(id sqltypes.Value) ([]byte, error) { return keybytes[:], nil } +// UnknownParams implements the ParamValidating interface. +func (vind *NumericStaticMap) UnknownParams() []string { + return vind.unknownParams +} + func loadNumericLookupTable(path string) (NumericLookupTable, error) { data, err := os.ReadFile(path) if err != nil { diff --git a/go/vt/vtgate/vindexes/numeric_static_map_test.go b/go/vt/vtgate/vindexes/numeric_static_map_test.go index 05815bd73b1..d6e3f5b38c3 100644 --- a/go/vt/vtgate/vindexes/numeric_static_map_test.go +++ b/go/vt/vtgate/vindexes/numeric_static_map_test.go @@ -18,6 +18,7 @@ package vindexes import ( "context" + "errors" "reflect" "testing" @@ -26,6 +27,8 @@ import ( "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" + "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" ) // createVindex creates the "numeric_static_map" vindex object which is used by @@ -40,23 +43,105 @@ func createVindex() (SingleColumn, error) { return vindex.(SingleColumn), nil } -// createVindexWithParams creates the "numeric_static_map" vindex object with the -// provided params. -func createVindexWithParams(params map[string]string) (SingleColumn, error) { - vindex, err := CreateVindex("numeric_static_map", "numericStaticMapWithParams", params) - if err != nil { - return nil, err +func numericStaticMapCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "numeric_static_map", + vindexName: "numeric_static_map", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "numeric_static_map", + expectUnknownParams: expectUnknownParams, } - return vindex.(SingleColumn), nil } -func TestNumericStaticMapInfo(t *testing.T) { - numericStaticMap, err := createVindex() - require.NoError(t, err) - assert.Equal(t, 1, numericStaticMap.Cost()) - assert.Equal(t, "numericStaticMap", numericStaticMap.String()) - assert.True(t, numericStaticMap.IsUnique()) - assert.False(t, numericStaticMap.NeedsVCursor()) +func TestNumericStaticMapCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + numericStaticMapCreateVindexTestCase( + "no params invalid, require either json_path or json", + nil, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "NumericStaticMap: Could not find either `json_path` or `json` params in vschema"), + nil, + ), + numericStaticMapCreateVindexTestCase( + "empty params invalid, require either json_path or json", + map[string]string{}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "NumericStaticMap: Could not find either `json_path` or `json` params in vschema"), + nil, + ), + numericStaticMapCreateVindexTestCase( + "json_path and json mutually exclusive", + map[string]string{ + "json": "{}", + "json_path": "/path/to/map.json", + }, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "NumericStaticMap: Found both `json` and `json_path` params in vschema"), + nil, + ), + numericStaticMapCreateVindexTestCase( + "json_path must exist", + map[string]string{ + "json_path": "/path/to/map.json", + }, + errors.New("open /path/to/map.json: no such file or directory"), + nil, + ), + numericStaticMapCreateVindexTestCase( + "json ok", + map[string]string{ + "json": "{}", + }, + nil, + nil, + ), + numericStaticMapCreateVindexTestCase( + "json must be valid syntax", + map[string]string{ + "json": "{]", + }, + errors.New("invalid character ']' looking for beginning of object key string"), + nil, + ), + numericStaticMapCreateVindexTestCase( + "fallback_type ok", + map[string]string{ + "json": "{}", + "fallback_type": "binary", + }, + nil, + nil, + ), + numericStaticMapCreateVindexTestCase( + "fallback_type must be valid vindex type", + map[string]string{ + "json": "{}", + "fallback_type": "not_found", + }, + vterrors.Errorf(vtrpc.Code_NOT_FOUND, "vindexType %q not found", "not_found"), + nil, + ), + numericStaticMapCreateVindexTestCase( + "unknown params", + map[string]string{ + "json": "{}", + "hello": "world", + }, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestNumericStaticMapMap(t *testing.T) { @@ -115,39 +200,62 @@ func TestNumericStaticMapVerify(t *testing.T) { } func TestNumericStaticMapWithJsonVdx(t *testing.T) { - withFallbackVdx, err := createVindexWithParams(map[string]string{ - "json": "{\"1\":2,\"3\":4,\"5\":6}", - }) + withFallbackVdx, err := CreateVindex( + "numeric_static_map", + t.Name(), + map[string]string{ + "json": "{\"1\":2,\"3\":4,\"5\":6}", + }, + ) require.NoError(t, err) + require.Empty(t, withFallbackVdx.(ParamValidating).UnknownParams()) assert.Equal(t, 1, withFallbackVdx.Cost()) - assert.Equal(t, "numericStaticMapWithParams", withFallbackVdx.String()) + assert.Equal(t, t.Name(), withFallbackVdx.String()) assert.True(t, withFallbackVdx.IsUnique()) assert.False(t, withFallbackVdx.NeedsVCursor()) // Bad format tests - _, err = createVindexWithParams(map[string]string{ - "json": "{\"1\":2,\"3\":4,\"5\":6:8,\"10\":11}", - }) + _, err = CreateVindex( + "numeric_static_map", + t.Name(), + map[string]string{ + "json": "{\"1\":2,\"3\":4,\"5\":6:8,\"10\":11}", + }, + ) require.EqualError(t, err, "invalid character ':' after object key:value pair") // Letters in key or value not allowed - _, err = createVindexWithParams(map[string]string{"json": "{\"1\":a}"}) + _, err = CreateVindex( + "numeric_static_map", + t.Name(), + map[string]string{"json": "{\"1\":a}"}, + ) require.EqualError(t, err, "invalid character 'a' looking for beginning of value") - _, err = createVindexWithParams(map[string]string{"json": "{\"a\":1}"}) + _, err = CreateVindex( + "numeric_static_map", + t.Name(), + map[string]string{"json": "{\"a\":1}"}, + ) require.EqualError(t, err, "strconv.ParseUint: parsing \"a\": invalid syntax") } // Test mapping of vindex, both for specified map keys and underlying xxhash func TestNumericStaticMapWithFallback(t *testing.T) { - mapWithFallbackVdx, err := createVindexWithParams(map[string]string{ - "json": "{\"1\":2,\"3\":4,\"4\":5,\"5\":6,\"6\":7,\"7\":8,\"8\":9,\"10\":18446744073709551615}", - "fallback_type": "xxhash", - }) + mapWithFallbackVdx, err := CreateVindex( + "numeric_static_map", + t.Name(), + map[string]string{ + "json": "{\"1\":2,\"3\":4,\"4\":5,\"5\":6,\"6\":7,\"7\":8,\"8\":9,\"10\":18446744073709551615}", + "fallback_type": "xxhash", + }, + ) if err != nil { t.Fatalf("failed to create vindex: %v", err) } - got, err := mapWithFallbackVdx.Map(context.Background(), nil, []sqltypes.Value{ + require.Empty(t, mapWithFallbackVdx.(ParamValidating).UnknownParams()) + singleCol := mapWithFallbackVdx.(SingleColumn) + got, err := singleCol.Map(context.Background(), nil, []sqltypes.Value{ sqltypes.NewInt64(1), sqltypes.NewInt64(2), sqltypes.NewInt64(3), @@ -183,14 +291,20 @@ func TestNumericStaticMapWithFallback(t *testing.T) { } func TestNumericStaticMapWithFallbackVerify(t *testing.T) { - mapWithFallbackVdx, err := createVindexWithParams(map[string]string{ - "json": "{\"1\":2,\"3\":4,\"4\":5,\"5\":6,\"6\":7,\"7\":8,\"8\":9,\"10\":18446744073709551615}", - "fallback_type": "xxhash", - }) + mapWithFallbackVdx, err := CreateVindex( + "numeric_static_map", + t.Name(), + map[string]string{ + "json": "{\"1\":2,\"3\":4,\"4\":5,\"5\":6,\"6\":7,\"7\":8,\"8\":9,\"10\":18446744073709551615}", + "fallback_type": "xxhash", + }, + ) if err != nil { t.Fatalf("failed to create vindex: %v", err) } - got, err := mapWithFallbackVdx.Verify(context.Background(), nil, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2), sqltypes.NewInt64(11), sqltypes.NewInt64(10)}, [][]byte{[]byte("\x00\x00\x00\x00\x00\x00\x00\x02"), []byte("\x8b\x59\x80\x16\x62\xb5\x21\x60"), []byte("\xff\xff\xff\xff\xff\xff\xff\xff"), []byte("\xff\xff\xff\xff\xff\xff\xff\xff")}) + require.Empty(t, mapWithFallbackVdx.(ParamValidating).UnknownParams()) + singleCol := mapWithFallbackVdx.(SingleColumn) + got, err := singleCol.Verify(context.Background(), nil, []sqltypes.Value{sqltypes.NewInt64(1), sqltypes.NewInt64(2), sqltypes.NewInt64(11), sqltypes.NewInt64(10)}, [][]byte{[]byte("\x00\x00\x00\x00\x00\x00\x00\x02"), []byte("\x8b\x59\x80\x16\x62\xb5\x21\x60"), []byte("\xff\xff\xff\xff\xff\xff\xff\xff"), []byte("\xff\xff\xff\xff\xff\xff\xff\xff")}) require.NoError(t, err) want := []bool{true, true, false, true} if !reflect.DeepEqual(got, want) { @@ -198,6 +312,6 @@ func TestNumericStaticMapWithFallbackVerify(t *testing.T) { } // Failure test - _, err = mapWithFallbackVdx.Verify(context.Background(), nil, []sqltypes.Value{sqltypes.NewVarBinary("aa")}, [][]byte{nil}) + _, err = singleCol.Verify(context.Background(), nil, []sqltypes.Value{sqltypes.NewVarBinary("aa")}, [][]byte{nil}) require.EqualError(t, err, "could not parse value: 'aa'") } diff --git a/go/vt/vtgate/vindexes/numeric_test.go b/go/vt/vtgate/vindexes/numeric_test.go index 5d317d3a161..974d589dca6 100644 --- a/go/vt/vtgate/vindexes/numeric_test.go +++ b/go/vt/vtgate/vindexes/numeric_test.go @@ -21,7 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -31,15 +30,58 @@ import ( var numeric SingleColumn func init() { - vindex, _ := CreateVindex("numeric", "num", nil) + vindex, err := CreateVindex("numeric", "num", nil) + if err != nil { + panic(err) + } numeric = vindex.(SingleColumn) } -func TestNumericInfo(t *testing.T) { - assert.Equal(t, 0, numeric.Cost()) - assert.Equal(t, "num", numeric.String()) - assert.True(t, numeric.IsUnique()) - assert.False(t, numeric.NeedsVCursor()) +func numericCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "numeric", + vindexName: "numeric", + vindexParams: vindexParams, + + expectCost: 0, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "numeric", + expectUnknownParams: expectUnknownParams, + } +} + +func TestNumericCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + numericCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + numericCreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + numericCreateVindexTestCase( + "unknown params", + map[string]string{"hello": "world"}, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestNumericMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/region_experimental.go b/go/vt/vtgate/vindexes/region_experimental.go index 14347e9d9a4..676a38e5c77 100644 --- a/go/vt/vtgate/vindexes/region_experimental.go +++ b/go/vt/vtgate/vindexes/region_experimental.go @@ -22,18 +22,29 @@ import ( "encoding/binary" "fmt" + "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" ) +const ( + regionExperimentalParamRegionBytes = "region_bytes" +) + var ( - _ MultiColumn = (*RegionExperimental)(nil) + _ MultiColumn = (*RegionExperimental)(nil) + _ ParamValidating = (*RegionExperimental)(nil) + + regionExperimentalParams = []string{ + regionExperimentalParamRegionBytes, + } ) func init() { - Register("region_experimental", NewRegionExperimental) + Register("region_experimental", newRegionExperimental) } // RegionExperimental is a multi-column unique vindex. The first column is prefixed @@ -41,17 +52,18 @@ func init() { // RegionExperimental can be used for geo-partitioning because the first column can denote a region, // and its value will dictate the shard for that region. type RegionExperimental struct { - name string - regionBytes int + name string + regionBytes int + unknownParams []string } -// NewRegionExperimental creates a RegionExperimental vindex. +// newRegionExperimental creates a RegionExperimental vindex. // The supplied map requires all the fields of "consistent_lookup_unique". // Additionally, it requires a region_bytes argument whose value can be "1", or "2". -func NewRegionExperimental(name string, m map[string]string) (Vindex, error) { - rbs, ok := m["region_bytes"] +func newRegionExperimental(name string, m map[string]string) (Vindex, error) { + rbs, ok := m[regionExperimentalParamRegionBytes] if !ok { - return nil, fmt.Errorf("region_experimental missing region_bytes param") + return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, fmt.Sprintf("region_experimental missing %s param", regionExperimentalParamRegionBytes)) } var rb int switch rbs { @@ -60,11 +72,12 @@ func NewRegionExperimental(name string, m map[string]string) (Vindex, error) { case "2": rb = 2 default: - return nil, fmt.Errorf("region_bits must be 1 or 2: %v", rbs) + return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "region_bytes must be 1 or 2: %v", rbs) } return &RegionExperimental{ - name: name, - regionBytes: rb, + name: name, + regionBytes: rb, + unknownParams: FindUnknownParams(m, regionExperimentalParams), }, nil } @@ -145,3 +158,8 @@ func (ge *RegionExperimental) Verify(ctx context.Context, vcursor VCursor, rowsC func (ge *RegionExperimental) PartialVindex() bool { return true } + +// UnknownParams implements the ParamValidating interface. +func (ge *RegionExperimental) UnknownParams() []string { + return ge.unknownParams +} diff --git a/go/vt/vtgate/vindexes/region_experimental_test.go b/go/vt/vtgate/vindexes/region_experimental_test.go index dde9a2f6ea9..56b16b8f3ee 100644 --- a/go/vt/vtgate/vindexes/region_experimental_test.go +++ b/go/vt/vtgate/vindexes/region_experimental_test.go @@ -22,6 +22,8 @@ import ( "testing" topodatapb "vitess.io/vitess/go/vt/proto/topodata" + "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,6 +32,88 @@ import ( "vitess.io/vitess/go/vt/key" ) +func regionExperimentalCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "region_experimental", + vindexName: "region_experimental", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "region_experimental", + expectUnknownParams: expectUnknownParams, + } +} + +func TestRegionExperimentalCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + regionExperimentalCreateVindexTestCase( + "no params invalid: region_bytes required", + nil, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "region_experimental missing region_bytes param"), + nil, + ), + regionExperimentalCreateVindexTestCase( + "empty params invalid: region_bytes required", + map[string]string{}, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "region_experimental missing region_bytes param"), + nil, + ), + regionExperimentalCreateVindexTestCase( + "region_bytes may not be 0", + map[string]string{ + "region_bytes": "0", + }, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "region_bytes must be 1 or 2: 0"), + nil, + ), + regionExperimentalCreateVindexTestCase( + "region_bytes may be 1", + map[string]string{ + "region_bytes": "1", + }, + nil, + nil, + ), + regionExperimentalCreateVindexTestCase( + "region_bytes may be 2", + map[string]string{ + "region_bytes": "2", + }, + nil, + nil, + ), + regionExperimentalCreateVindexTestCase( + "region_bytes may not be 3", + map[string]string{ + "region_bytes": "3", + }, + vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "region_bytes must be 1 or 2: 3"), + nil, + ), + regionExperimentalCreateVindexTestCase( + "unknown params", + map[string]string{ + "region_bytes": "1", + "hello": "world", + }, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) +} + func TestRegionExperimentalMisc(t *testing.T) { ge, err := createRegionVindex(t, "region_experimental", "f1,f2", 1) require.NoError(t, err) @@ -122,18 +206,8 @@ func TestRegionExperimentalVerifyMulti(t *testing.T) { assert.Equal(t, want, got) } -func TestRegionExperimentalCreateErrors(t *testing.T) { - _, err := createRegionVindex(t, "region_experimental", "f1,f2", 3) - assert.EqualError(t, err, "region_bits must be 1 or 2: 3") - _, err = CreateVindex("region_experimental", "region_experimental", nil) - assert.EqualError(t, err, "region_experimental missing region_bytes param") -} - func createRegionVindex(t *testing.T, name, from string, rb int) (Vindex, error) { return CreateVindex(name, name, map[string]string{ "region_bytes": strconv.Itoa(rb), - "table": "t", - "from": from, - "to": "toc", }) } diff --git a/go/vt/vtgate/vindexes/region_json.go b/go/vt/vtgate/vindexes/region_json.go index 093ccd9090b..1a28354261a 100644 --- a/go/vt/vtgate/vindexes/region_json.go +++ b/go/vt/vtgate/vindexes/region_json.go @@ -32,12 +32,22 @@ import ( "vitess.io/vitess/go/vt/log" ) +const ( + regionJSONParamRegionBytes = "region_bytes" + regionJSONParamRegionMap = "region_map" +) + var ( _ MultiColumn = (*RegionJSON)(nil) + + regionJSONParams = []string{ + regionJSONParamRegionBytes, + regionJSONParamRegionMap, + } ) func init() { - Register("region_json", NewRegionJSON) + Register("region_json", newRegionJSON) } // RegionMap is used to store mapping of country to region @@ -49,17 +59,18 @@ type RegionMap map[string]uint64 // RegionJson can be used for geo-partitioning because the first column can denote a region, // and it will dictate the shard range for that region. type RegionJSON struct { - name string - regionMap RegionMap - regionBytes int + name string + regionMap RegionMap + regionBytes int + unknownParams []string } -// NewRegionJSON creates a RegionJson vindex. +// newRegionJSON creates a RegionJson vindex. // The supplied map requires all the fields of "RegionExperimental". // Additionally, it requires a region_map argument representing the path to a json file // containing a map of country to region. -func NewRegionJSON(name string, m map[string]string) (Vindex, error) { - rmPath := m["region_map"] +func newRegionJSON(name string, m map[string]string) (Vindex, error) { + rmPath := m[regionJSONParamRegionMap] rmap := make(map[string]uint64) data, err := os.ReadFile(rmPath) if err != nil { @@ -70,7 +81,7 @@ func NewRegionJSON(name string, m map[string]string) (Vindex, error) { if err != nil { return nil, err } - rb, err := strconv.Atoi(m["region_bytes"]) + rb, err := strconv.Atoi(m[regionJSONParamRegionBytes]) if err != nil { return nil, err } @@ -81,9 +92,10 @@ func NewRegionJSON(name string, m map[string]string) (Vindex, error) { } return &RegionJSON{ - name: name, - regionMap: rmap, - regionBytes: rb, + name: name, + regionMap: rmap, + regionBytes: rb, + unknownParams: FindUnknownParams(m, regionJSONParams), }, nil } @@ -158,3 +170,8 @@ func (rv *RegionJSON) Verify(ctx context.Context, vcursor VCursor, rowsColValues func (rv *RegionJSON) PartialVindex() bool { return false } + +// UnknownParams implements the ParamValidating interface. +func (rv *RegionJSON) UnknownParams() []string { + return rv.unknownParams +} diff --git a/go/vt/vtgate/vindexes/reverse_bits.go b/go/vt/vtgate/vindexes/reverse_bits.go index 332cae5dfce..184e62de5a0 100644 --- a/go/vt/vtgate/vindexes/reverse_bits.go +++ b/go/vt/vtgate/vindexes/reverse_bits.go @@ -39,12 +39,16 @@ var ( // ReverseBits defines vindex that reverses the bits of a number. // It's Unique, Reversible and Functional. type ReverseBits struct { - name string + name string + unknownParams []string } -// NewReverseBits creates a new ReverseBits. -func NewReverseBits(name string, m map[string]string) (Vindex, error) { - return &ReverseBits{name: name}, nil +// newReverseBits creates a new ReverseBits. +func newReverseBits(name string, m map[string]string) (Vindex, error) { + return &ReverseBits{ + name: name, + unknownParams: FindUnknownParams(m, nil), + }, nil } // String returns the name of the vindex. @@ -107,6 +111,11 @@ func (vind *ReverseBits) ReverseMap(_ VCursor, ksids [][]byte) ([]sqltypes.Value return reverseIds, nil } +// UnknownParams implements the ParamValidating interface. +func (vind *ReverseBits) UnknownParams() []string { + return vind.unknownParams +} + func (vind *ReverseBits) Hash(id sqltypes.Value) ([]byte, error) { num, err := evalengine.ToUint64(id) if err != nil { @@ -116,7 +125,7 @@ func (vind *ReverseBits) Hash(id sqltypes.Value) ([]byte, error) { } func init() { - Register("reverse_bits", NewReverseBits) + Register("reverse_bits", newReverseBits) } func reverse(shardKey uint64) []byte { diff --git a/go/vt/vtgate/vindexes/reverse_bits_test.go b/go/vt/vtgate/vindexes/reverse_bits_test.go index 14f3d59820a..9f5d2b56b15 100644 --- a/go/vt/vtgate/vindexes/reverse_bits_test.go +++ b/go/vt/vtgate/vindexes/reverse_bits_test.go @@ -21,7 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -31,18 +30,64 @@ import ( var reverseBits SingleColumn func init() { - hv, err := CreateVindex("reverse_bits", "rr", map[string]string{"Table": "t", "Column": "c"}) + hv, err := CreateVindex("reverse_bits", "rr", map[string]string{}) if err != nil { panic(err) } + unknownParams := hv.(ParamValidating).UnknownParams() + if len(unknownParams) > 0 { + panic("reverse_bits test init: expected 0 unknown params") + } reverseBits = hv.(SingleColumn) } -func TestReverseBitsInfo(t *testing.T) { - assert.Equal(t, 1, reverseBits.Cost()) - assert.Equal(t, "rr", reverseBits.String()) - assert.True(t, reverseBits.IsUnique()) - assert.False(t, reverseBits.NeedsVCursor()) +func reverseBitsCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "reverse_bits", + vindexName: "reverse_bits", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "reverse_bits", + expectUnknownParams: expectUnknownParams, + } +} + +func TestReverseBitsCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + reverseBitsCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + reverseBitsCreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + reverseBitsCreateVindexTestCase( + "unknown params", + map[string]string{ + "hello": "world", + }, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestReverseBitsMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/unicodeloosemd5.go b/go/vt/vtgate/vindexes/unicodeloosemd5.go index dfe7d59f737..8fa6ac33bef 100644 --- a/go/vt/vtgate/vindexes/unicodeloosemd5.go +++ b/go/vt/vtgate/vindexes/unicodeloosemd5.go @@ -26,8 +26,9 @@ import ( ) var ( - _ SingleColumn = (*UnicodeLooseMD5)(nil) - _ Hashing = (*UnicodeLooseMD5)(nil) + _ SingleColumn = (*UnicodeLooseMD5)(nil) + _ Hashing = (*UnicodeLooseMD5)(nil) + _ ParamValidating = (*UnicodeLooseMD5)(nil) ) // UnicodeLooseMD5 is a vindex that normalizes and hashes unicode strings @@ -36,12 +37,16 @@ var ( // Ref: http://www.unicode.org/reports/tr10/#Multi_Level_Comparison. // This is compatible with MySQL's utf8_unicode_ci collation. type UnicodeLooseMD5 struct { - name string + name string + unknownParams []string } -// NewUnicodeLooseMD5 creates a new UnicodeLooseMD5. -func NewUnicodeLooseMD5(name string, _ map[string]string) (Vindex, error) { - return &UnicodeLooseMD5{name: name}, nil +// newUnicodeLooseMD5 creates a new UnicodeLooseMD5. +func newUnicodeLooseMD5(name string, m map[string]string) (Vindex, error) { + return &UnicodeLooseMD5{ + name: name, + unknownParams: FindUnknownParams(m, nil), + }, nil } // String returns the name of the vindex. @@ -94,6 +99,11 @@ func (vind *UnicodeLooseMD5) Hash(id sqltypes.Value) ([]byte, error) { return unicodeHash(vMD5Hash, id) } +// UnknownParams implements the ParamValidating interface. +func (vind *UnicodeLooseMD5) UnknownParams() []string { + return vind.unknownParams +} + func init() { - Register("unicode_loose_md5", NewUnicodeLooseMD5) + Register("unicode_loose_md5", newUnicodeLooseMD5) } diff --git a/go/vt/vtgate/vindexes/unicodeloosemd5_test.go b/go/vt/vtgate/vindexes/unicodeloosemd5_test.go index dea4a048783..879414c5be9 100644 --- a/go/vt/vtgate/vindexes/unicodeloosemd5_test.go +++ b/go/vt/vtgate/vindexes/unicodeloosemd5_test.go @@ -21,8 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" ) @@ -30,15 +28,60 @@ import ( var charVindexMD5 SingleColumn func init() { - vindex, _ := CreateVindex("unicode_loose_md5", "utf8ch", nil) + vindex, err := CreateVindex("unicode_loose_md5", "utf8ch", nil) + if err != nil { + panic(err) + } charVindexMD5 = vindex.(SingleColumn) } -func TestUnicodeLooseMD5Info(t *testing.T) { - assert.Equal(t, 1, charVindexMD5.Cost()) - assert.Equal(t, "utf8ch", charVindexMD5.String()) - assert.True(t, charVindexMD5.IsUnique()) - assert.False(t, charVindexMD5.NeedsVCursor()) +func unicodeLooseMD5CreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "unicode_loose_md5", + vindexName: "unicode_loose_md5", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "unicode_loose_md5", + expectUnknownParams: expectUnknownParams, + } +} + +func TestUnicodeLooseMD5CreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + unicodeLooseMD5CreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + unicodeLooseMD5CreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + unicodeLooseMD5CreateVindexTestCase( + "unknown params", + map[string]string{ + "hello": "world", + }, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestUnicodeLooseMD5Map(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/unicodeloosexxhash.go b/go/vt/vtgate/vindexes/unicodeloosexxhash.go index dcd924131aa..5e04bff1866 100644 --- a/go/vt/vtgate/vindexes/unicodeloosexxhash.go +++ b/go/vt/vtgate/vindexes/unicodeloosexxhash.go @@ -26,8 +26,9 @@ import ( ) var ( - _ SingleColumn = (*UnicodeLooseXXHash)(nil) - _ Hashing = (*UnicodeLooseXXHash)(nil) + _ SingleColumn = (*UnicodeLooseXXHash)(nil) + _ Hashing = (*UnicodeLooseXXHash)(nil) + _ ParamValidating = (*UnicodeLooseXXHash)(nil) ) // UnicodeLooseXXHash is a vindex that normalizes and hashes unicode strings @@ -36,12 +37,16 @@ var ( // Ref: http://www.unicode.org/reports/tr10/#Multi_Level_Comparison. // This is compatible with MySQL's utf8_unicode_ci collation. type UnicodeLooseXXHash struct { - name string + name string + unknownParams []string } -// NewUnicodeLooseXXHash creates a new UnicodeLooseXXHash struct. -func NewUnicodeLooseXXHash(name string, _ map[string]string) (Vindex, error) { - return &UnicodeLooseXXHash{name: name}, nil +// newUnicodeLooseXXHash creates a new UnicodeLooseXXHash struct. +func newUnicodeLooseXXHash(name string, m map[string]string) (Vindex, error) { + return &UnicodeLooseXXHash{ + name: name, + unknownParams: FindUnknownParams(m, nil), + }, nil } // String returns the name of the vindex. @@ -94,6 +99,11 @@ func (vind *UnicodeLooseXXHash) Hash(id sqltypes.Value) ([]byte, error) { return unicodeHash(vXXHash, id) } +// UnknownParams implements the ParamValidating interface. +func (vind *UnicodeLooseXXHash) UnknownParams() []string { + return vind.unknownParams +} + func init() { - Register("unicode_loose_xxhash", NewUnicodeLooseXXHash) + Register("unicode_loose_xxhash", newUnicodeLooseXXHash) } diff --git a/go/vt/vtgate/vindexes/unicodeloosexxhash_test.go b/go/vt/vtgate/vindexes/unicodeloosexxhash_test.go index e5ae98cf87f..6836bfd4ffa 100644 --- a/go/vt/vtgate/vindexes/unicodeloosexxhash_test.go +++ b/go/vt/vtgate/vindexes/unicodeloosexxhash_test.go @@ -21,8 +21,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" ) @@ -30,15 +28,64 @@ import ( var charVindexXXHash SingleColumn func init() { - vindex, _ := CreateVindex("unicode_loose_xxhash", "utf8ch", nil) + vindex, err := CreateVindex("unicode_loose_xxhash", "utf8ch", nil) + if err != nil { + panic(err) + } + unknownParams := vindex.(ParamValidating).UnknownParams() + if len(unknownParams) > 0 { + panic("unicode_loose_xxhash test init: expected 0 unknown params") + } charVindexXXHash = vindex.(SingleColumn) } -func TestUnicodeLooseXXHashInfo(t *testing.T) { - assert.Equal(t, 1, charVindexXXHash.Cost()) - assert.Equal(t, "utf8ch", charVindexXXHash.String()) - assert.True(t, charVindexXXHash.IsUnique()) - assert.False(t, charVindexXXHash.NeedsVCursor()) +func unicodeLooseXXHashCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "unicode_loose_xxhash", + vindexName: "unicode_loose_xxhash", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "unicode_loose_xxhash", + expectUnknownParams: expectUnknownParams, + } +} + +func TestUnicodeLooseXXHashCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + unicodeLooseXXHashCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + unicodeLooseXXHashCreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + unicodeLooseXXHashCreateVindexTestCase( + "unknown params", + map[string]string{ + "hello": "world", + }, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestUnicodeLooseXXHashMap(t *testing.T) { diff --git a/go/vt/vtgate/vindexes/vindex.go b/go/vt/vtgate/vindexes/vindex.go index 700b8e6175c..141e1e61efe 100644 --- a/go/vt/vtgate/vindexes/vindex.go +++ b/go/vt/vtgate/vindexes/vindex.go @@ -19,6 +19,7 @@ package vindexes import ( "context" "fmt" + "sort" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" @@ -71,6 +72,22 @@ type ( NeedsVCursor() bool } + // ParamValidating is an optional interface that Vindexes may implement to + // report errors about unknown params encountered during Vindex creation. + ParamValidating interface { + // UnknownParams returns a slice of param names that were provided + // during Vindex creation, but were not known and therefore ignored by + // the Vindex. + UnknownParams() []string + } + + // ParamValidationOpts may be used by Vindexes that accept params to + // validate params with ValidateParams(params, opts). + ParamValidationOpts struct { + // Params contains param names known by the vindex. + Params []string + } + // SingleColumn defines the interface for a single column vindex. SingleColumn interface { Vindex @@ -166,7 +183,7 @@ type ( var registry = make(map[string]NewVindexFunc) -// Register registers a vindex under the specified vindexType. +// Register registers a vindex factory under the specified vindexType. // A duplicate vindexType will generate a panic. // New vindexes will be created using these functions at the // time of vschema loading. @@ -179,7 +196,7 @@ func Register(vindexType string, newVindexFunc NewVindexFunc) { // CreateVindex creates a vindex of the specified type using the // supplied params. The type must have been previously registered. -func CreateVindex(vindexType, name string, params map[string]string) (Vindex, error) { +func CreateVindex(vindexType, name string, params map[string]string) (vindex Vindex, err error) { f, ok := registry[vindexType] if !ok { return nil, fmt.Errorf("vindexType %q not found", vindexType) @@ -216,3 +233,19 @@ func firstColsOnly(rowsColValues [][]sqltypes.Value) []sqltypes.Value { } return firstCols } + +// FindUnknownParams a sorted slice of keys in params that are not present in knownParams. +func FindUnknownParams(params map[string]string, knownParams []string) []string { + var unknownParams []string + knownParamsByName := make(map[string]struct{}) + for _, knownParam := range knownParams { + knownParamsByName[knownParam] = struct{}{} + } + for name := range params { + if _, ok := knownParamsByName[name]; !ok { + unknownParams = append(unknownParams, name) + } + } + sort.Strings(unknownParams) + return unknownParams +} diff --git a/go/vt/vtgate/vindexes/vindex_test.go b/go/vt/vtgate/vindexes/vindex_test.go index 6e2952e6bca..97b17da2a4b 100644 --- a/go/vt/vtgate/vindexes/vindex_test.go +++ b/go/vt/vtgate/vindexes/vindex_test.go @@ -18,14 +18,68 @@ package vindexes import ( "context" + "sort" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" ) +type testVindex struct { + allowUnknownParams bool + knownParams []string + params map[string]string +} + +func (v *testVindex) Cost() int { + return 0 +} + +func (v *testVindex) String() string { + return "" +} + +func (v *testVindex) IsUnique() bool { + return false +} + +func (v *testVindex) NeedsVCursor() bool { + return false +} + +func (v *testVindex) UnknownParams() []string { + if v.allowUnknownParams { + return nil + } + return FindUnknownParams(v.params, v.knownParams) +} + +func init() { + Register("allow_unknown_params", func(_ string, params map[string]string) (Vindex, error) { + return &testVindex{ + allowUnknownParams: true, + knownParams: []string{ + "option1", + "option2", + }, + params: params, + }, nil + }) + Register("warn_unknown_params", func(_ string, params map[string]string) (Vindex, error) { + return &testVindex{ + allowUnknownParams: false, + knownParams: []string{ + "option1", + "option2", + }, + params: params, + }, nil + }) +} + func TestVindexMap(t *testing.T) { ge, err := createRegionVindex(t, "region_experimental", "f1,f2", 1) assert.NoError(t, err) @@ -42,6 +96,7 @@ func TestVindexMap(t *testing.T) { hash, err := CreateVindex("hash", "hash", nil) assert.NoError(t, err) + require.Empty(t, hash.(ParamValidating).UnknownParams()) got, err = Map(context.Background(), hash, nil, [][]sqltypes.Value{{ sqltypes.NewInt64(1), }}) @@ -55,6 +110,7 @@ func TestVindexMap(t *testing.T) { func TestVindexVerify(t *testing.T) { ge, err := createRegionVindex(t, "region_experimental", "f1,f2", 1) assert.NoError(t, err) + require.Empty(t, ge.(ParamValidating).UnknownParams()) got, err := Verify(context.Background(), ge, nil, [][]sqltypes.Value{{ sqltypes.NewInt64(1), sqltypes.NewInt64(1), @@ -67,6 +123,7 @@ func TestVindexVerify(t *testing.T) { assert.Equal(t, want, got) hash, err := CreateVindex("hash", "hash", nil) + require.Empty(t, hash.(ParamValidating).UnknownParams()) assert.NoError(t, err) got, err = Verify(context.Background(), hash, nil, [][]sqltypes.Value{{ sqltypes.NewInt64(1), @@ -76,3 +133,40 @@ func TestVindexVerify(t *testing.T) { assert.NoError(t, err) assert.Equal(t, want, got) } + +func TestCreateVindexAllowUnknownParams(t *testing.T) { + vindex, err := CreateVindex( + "allow_unknown_params", + "allow_unknown_params", + map[string]string{ + "option1": "value1", + "option2": "value2", + "option3": "value3", + "option4": "value4", + }, + ) + + require.NotNil(t, vindex) + require.NoError(t, err) +} + +func TestCreateVindexWarnUnknownParams(t *testing.T) { + vindex, err := CreateVindex( + "warn_unknown_params", + "warn_unknown_params", + map[string]string{ + "option1": "value1", + "option2": "value2", + "option3": "value3", + "option4": "value4", + }, + ) + + require.NotNil(t, vindex) + require.NoError(t, err) + + unknownParams := vindex.(ParamValidating).UnknownParams() + sort.Strings(unknownParams) + require.Len(t, unknownParams, 2) + require.Equal(t, []string{"option3", "option4"}, unknownParams) +} diff --git a/go/vt/vtgate/vindexes/vschema_test.go b/go/vt/vtgate/vindexes/vschema_test.go index c4684cc8945..335b42560cb 100644 --- a/go/vt/vtgate/vindexes/vschema_test.go +++ b/go/vt/vtgate/vindexes/vschema_test.go @@ -41,8 +41,7 @@ import ( // cheapVindex is a Functional, Unique Vindex. type cheapVindex struct { - name string - Params map[string]string + name string } func (v *cheapVindex) String() string { return v.name } @@ -56,16 +55,15 @@ func (*cheapVindex) Map(ctx context.Context, vcursor VCursor, ids []sqltypes.Val return nil, nil } -func NewCheapVindex(name string, params map[string]string) (Vindex, error) { - return &cheapVindex{name: name, Params: params}, nil +func newCheapVindex(name string, _ map[string]string) (Vindex, error) { + return &cheapVindex{name: name}, nil } var _ SingleColumn = (*stFU)(nil) // stFU is a Functional, Unique Vindex. type stFU struct { - name string - Params map[string]string + name string } func (v *stFU) String() string { return v.name } @@ -79,16 +77,15 @@ func (*stFU) Map(ctx context.Context, vcursor VCursor, ids []sqltypes.Value) ([] return nil, nil } -func NewSTFU(name string, params map[string]string) (Vindex, error) { - return &stFU{name: name, Params: params}, nil +func newSTFU(name string, _ map[string]string) (Vindex, error) { + return &stFU{name: name}, nil } var _ SingleColumn = (*stFU)(nil) // stFN is a Functional, NonUnique Vindex. type stFN struct { - name string - Params map[string]string + name string } func (v *stFN) String() string { return v.name } @@ -102,16 +99,15 @@ func (*stFN) Map(ctx context.Context, vcursor VCursor, ids []sqltypes.Value) ([] return nil, nil } -func NewSTFN(name string, params map[string]string) (Vindex, error) { - return &stFN{name: name, Params: params}, nil +func newSTFN(name string, _ map[string]string) (Vindex, error) { + return &stFN{name: name}, nil } var _ SingleColumn = (*stFN)(nil) // stLN is a Lookup, NonUnique Vindex. type stLN struct { - name string - Params map[string]string + name string } func (v *stLN) String() string { return v.name } @@ -130,8 +126,8 @@ func (*stLN) Update(context.Context, VCursor, []sqltypes.Value, []byte, []sqltyp return nil } -func NewSTLN(name string, params map[string]string) (Vindex, error) { - return &stLN{name: name, Params: params}, nil +func newSTLN(name string, _ map[string]string) (Vindex, error) { + return &stLN{name: name}, nil } var _ SingleColumn = (*stLN)(nil) @@ -139,8 +135,7 @@ var _ Lookup = (*stLN)(nil) // stLU is a Lookup, Unique Vindex. type stLU struct { - name string - Params map[string]string + name string } func (v *stLU) String() string { return v.name } @@ -159,8 +154,8 @@ func (*stLU) Update(context.Context, VCursor, []sqltypes.Value, []byte, []sqltyp return nil } -func NewSTLU(name string, params map[string]string) (Vindex, error) { - return &stLU{name: name, Params: params}, nil +func newSTLU(name string, _ map[string]string) (Vindex, error) { + return &stLU{name: name}, nil } var _ SingleColumn = (*stLO)(nil) @@ -197,7 +192,7 @@ func (v *stLO) SetOwnerInfo(keyspace, table string, cols []sqlparser.IdentifierC return nil } -func NewSTLO(name string, _ map[string]string) (Vindex, error) { +func newSTLO(name string, _ map[string]string) (Vindex, error) { return &stLO{name: name}, nil } @@ -206,8 +201,7 @@ var _ Lookup = (*stLO)(nil) // mcFU is a multi-column Functional, Unique Vindex. type mcFU struct { - name string - Params map[string]string + name string } func (v *mcFU) String() string { return v.name } @@ -222,21 +216,21 @@ func (*mcFU) Map(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltype } func (*mcFU) PartialVindex() bool { return false } -func NewMCFU(name string, params map[string]string) (Vindex, error) { - return &mcFU{name: name, Params: params}, nil +func newMCFU(name string, _ map[string]string) (Vindex, error) { + return &mcFU{name: name}, nil } var _ MultiColumn = (*mcFU)(nil) func init() { - Register("cheap", NewCheapVindex) - Register("stfu", NewSTFU) - Register("stfn", NewSTFN) - Register("stln", NewSTLN) - Register("stlu", NewSTLU) - Register("stlo", NewSTLO) - Register("region_experimental_test", NewRegionExperimental) - Register("mcfu", NewMCFU) + Register("cheap", newCheapVindex) + Register("stfu", newSTFU) + Register("stfn", newSTFN) + Register("stln", newSTLN) + Register("stlu", newSTLU) + Register("stlo", newSTLO) + Register("region_experimental_test", newRegionExperimental) + Register("mcfu", newMCFU) } func TestUnshardedVSchemaValid(t *testing.T) { @@ -406,10 +400,9 @@ func TestShardedVSchemaOwned(t *testing.T) { Sharded: true, Vindexes: map[string]*vschemapb.Vindex{ "stfu1": { - Type: "stfu", - Params: map[string]string{ - "stfu1": "1"}, - Owner: "t1"}, + Type: "stfu", + Params: map[string]string{}, + Owner: "t1"}, "stln1": { Type: "stln", Owner: "t1"}}, @@ -429,10 +422,7 @@ func TestShardedVSchemaOwned(t *testing.T) { t1, err := got.FindTable("sharded", "t1") require.NoError(t, err) - vindex1 := &stFU{ - name: "stfu1", - Params: map[string]string{ - "stfu1": "1"}} + vindex1 := &stFU{name: "stfu1"} assertVindexMatches(t, t1.ColumnVindexes[0], vindex1, "stfu1", false) vindex2 := &stLN{name: "stln1"} @@ -876,12 +866,7 @@ func TestFindVindexForSharding(t *testing.T) { Name: "sharded", Sharded: true, } - vindex1 := &stFU{ - name: "stfu1", - Params: map[string]string{ - "stfu1": "1", - }, - } + vindex1 := &stFU{name: "stfu1"} vindex2 := &stLN{name: "stln1"} t1 := &Table{ Name: sqlparser.NewIdentifierCS("t1"), @@ -959,12 +944,7 @@ func TestFindVindexForSharding2(t *testing.T) { Sharded: true, } vindex1 := &stLU{name: "stlu1"} - vindex2 := &stFU{ - name: "stfu1", - Params: map[string]string{ - "stfu1": "1", - }, - } + vindex2 := &stFU{name: "stfu1"} t1 := &Table{ Name: sqlparser.NewIdentifierCS("t1"), Keyspace: ks, @@ -1002,10 +982,9 @@ func TestShardedVSchemaMultiColumnVindex(t *testing.T) { Sharded: true, Vindexes: map[string]*vschemapb.Vindex{ "stfu1": { - Type: "stfu", - Params: map[string]string{ - "stfu1": "1"}, - Owner: "t1"}}, + Type: "stfu", + Params: map[string]string{}, + Owner: "t1"}}, Tables: map[string]*vschemapb.Table{ "t1": { ColumnVindexes: []*vschemapb.ColumnVindex{{ @@ -1019,12 +998,7 @@ func TestShardedVSchemaMultiColumnVindex(t *testing.T) { Name: "sharded", Sharded: true, } - vindex1 := &stFU{ - name: "stfu1", - Params: map[string]string{ - "stfu1": "1", - }, - } + vindex1 := &stFU{name: "stfu1"} t1 := &Table{ Name: sqlparser.NewIdentifierCS("t1"), Keyspace: ks, @@ -1889,10 +1863,8 @@ func TestSequence(t *testing.T) { Sharded: true, Vindexes: map[string]*vschemapb.Vindex{ "stfu1": { - Type: "stfu", - Params: map[string]string{ - "stfu1": "1", - }, + Type: "stfu", + Params: map[string]string{}, }, }, Tables: map[string]*vschemapb.Table{ @@ -1943,12 +1915,7 @@ func TestSequence(t *testing.T) { Keyspace: ksu, Type: "sequence", } - vindex1 := &stFU{ - name: "stfu1", - Params: map[string]string{ - "stfu1": "1", - }, - } + vindex1 := &stFU{name: "stfu1"} t1 := &Table{ Name: sqlparser.NewIdentifierCS("t1"), Keyspace: kss, @@ -2168,10 +2135,8 @@ func TestFindTable(t *testing.T) { Sharded: true, Vindexes: map[string]*vschemapb.Vindex{ "stfu1": { - Type: "stfu", - Params: map[string]string{ - "stfu1": "1", - }, + Type: "stfu", + Params: map[string]string{}, }, }, Tables: map[string]*vschemapb.Table{ @@ -2552,11 +2517,15 @@ func TestVSchemaPBJSON(t *testing.T) { } func TestVSchemaJSON(t *testing.T) { - lkp, _ := NewLookupHash("n2", map[string]string{ + lkp, err := newLookupHash("n2", map[string]string{ "from": "f", "table": "t", "to": "2", }) + unknownParams := lkp.(ParamValidating).UnknownParams() + require.Empty(t, unknownParams) + require.NoError(t, err) + in := map[string]*KeyspaceSchema{ "unsharded": { Keyspace: &Keyspace{ diff --git a/go/vt/vtgate/vindexes/xxhash.go b/go/vt/vtgate/vindexes/xxhash.go index 471ad996757..3362cd0aab1 100644 --- a/go/vt/vtgate/vindexes/xxhash.go +++ b/go/vt/vtgate/vindexes/xxhash.go @@ -28,19 +28,24 @@ import ( ) var ( - _ SingleColumn = (*XXHash)(nil) - _ Hashing = (*XXHash)(nil) + _ SingleColumn = (*XXHash)(nil) + _ Hashing = (*XXHash)(nil) + _ ParamValidating = (*XXHash)(nil) ) // XXHash defines vindex that hashes any sql types to a KeyspaceId // by using xxhash64. It's Unique and works on any platform giving identical result. type XXHash struct { - name string + name string + unknownParams []string } -// NewXXHash creates a new XXHash. -func NewXXHash(name string, _ map[string]string) (Vindex, error) { - return &XXHash{name: name}, nil +// newXXHash creates a new XXHash. +func newXXHash(name string, m map[string]string) (Vindex, error) { + return &XXHash{ + name: name, + unknownParams: FindUnknownParams(m, nil), + }, nil } // String returns the name of the vindex. @@ -97,8 +102,13 @@ func (vind *XXHash) Hash(id sqltypes.Value) ([]byte, error) { return vXXHash(idBytes), nil } +// UnknownParams implements the ParamValidating interface. +func (vind *XXHash) UnknownParams() []string { + return vind.unknownParams +} + func init() { - Register("xxhash", NewXXHash) + Register("xxhash", newXXHash) } func vXXHash(shardKey []byte) []byte { diff --git a/go/vt/vtgate/vindexes/xxhash_test.go b/go/vt/vtgate/vindexes/xxhash_test.go index 148a2e7e7f8..b7bd77a1142 100644 --- a/go/vt/vtgate/vindexes/xxhash_test.go +++ b/go/vt/vtgate/vindexes/xxhash_test.go @@ -24,8 +24,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" ) @@ -33,18 +31,60 @@ import ( var xxHash SingleColumn func init() { - hv, err := CreateVindex("xxhash", "xxhash_name", map[string]string{"Table": "t", "Column": "c"}) + hv, err := CreateVindex("xxhash", "xxhash_name", map[string]string{}) if err != nil { panic(err) } xxHash = hv.(SingleColumn) } -func TestXXHashInfo(t *testing.T) { - assert.Equal(t, 1, xxHash.Cost()) - assert.Equal(t, "xxhash_name", xxHash.String()) - assert.True(t, xxHash.IsUnique()) - assert.False(t, xxHash.NeedsVCursor()) +func xxhashCreateVindexTestCase( + testName string, + vindexParams map[string]string, + expectErr error, + expectUnknownParams []string, +) createVindexTestCase { + return createVindexTestCase{ + testName: testName, + + vindexType: "xxhash", + vindexName: "xxhash", + vindexParams: vindexParams, + + expectCost: 1, + expectErr: expectErr, + expectIsUnique: true, + expectNeedsVCursor: false, + expectString: "xxhash", + expectUnknownParams: expectUnknownParams, + } +} + +func TestXXHashCreateVindex(t *testing.T) { + cases := []createVindexTestCase{ + xxhashCreateVindexTestCase( + "no params", + nil, + nil, + nil, + ), + xxhashCreateVindexTestCase( + "empty params", + map[string]string{}, + nil, + nil, + ), + xxhashCreateVindexTestCase( + "unknown params", + map[string]string{ + "hello": "world", + }, + nil, + []string{"hello"}, + ), + } + + testCreateVindexes(t, cases) } func TestXXHashMap(t *testing.T) { diff --git a/go/vt/vttablet/tabletserver/vstreamer/planbuilder_test.go b/go/vt/vttablet/tabletserver/vstreamer/planbuilder_test.go index 4ec57b15b7d..849ad2cec1a 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/planbuilder_test.go +++ b/go/vt/vttablet/tabletserver/vstreamer/planbuilder_test.go @@ -597,7 +597,7 @@ func TestPlanBuilderFilterComparison(t *testing.T) { Type: sqltypes.VarBinary, }}, } - hashVindex, err := vindexes.NewHash("hash", nil) + hashVindex, err := vindexes.CreateVindex("hash", "hash", nil) require.NoError(t, err) testcases := []struct { name string diff --git a/go/vt/wrangler/materializer_test.go b/go/vt/wrangler/materializer_test.go index 8f92a814f06..b2f90303802 100644 --- a/go/vt/wrangler/materializer_test.go +++ b/go/vt/wrangler/materializer_test.go @@ -1180,10 +1180,9 @@ func TestCreateCustomizedVindex(t *testing.T) { "v": { Type: "lookup_unique", Params: map[string]string{ - "table": "ks.lkp", - "from": "c1", - "to": "col2", - "data_type": "bigint(20)", + "table": "ks.lkp", + "from": "c1", + "to": "col2", }, Owner: "t1", }, @@ -1232,7 +1231,6 @@ func TestCreateCustomizedVindex(t *testing.T) { "table": "ks.lkp", "from": "c1", "to": "col2", - "data_type": "bigint(20)", "write_only": "true", }, Owner: "t1", @@ -1451,6 +1449,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { Params: map[string]string{ "table": "targetks.t", "from": "c1,c2", + "to": "c3", }, }, }, @@ -1465,6 +1464,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { Params: map[string]string{ "table": "targetks.t", "from": "c1", + "to": "c2", }, }, }, @@ -1479,6 +1479,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { Params: map[string]string{ "table": "targetks.t", "from": "c1,c2", + "to": "c2", }, }, }, @@ -1521,6 +1522,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { Params: map[string]string{ "table": "targetks.t", "from": "c1", + "to": "c2", }, Owner: "otherTable", }, @@ -1570,6 +1572,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { Params: map[string]string{ "table": "targetks.t", "from": "c1", + "to": "c2", }, Owner: "t1", }, @@ -2451,6 +2454,11 @@ func TestMaterializerNoGoodVindex(t *testing.T) { Vindexes: map[string]*vschemapb.Vindex{ "lookup_unique": { Type: "lookup_unique", + Params: map[string]string{ + "table": "t1", + "from": "c1", + "to": "c2", + }, }, }, Tables: map[string]*vschemapb.Table{