From 61c474a7e83b3f7ac17d4ad97851062f5c8469b3 Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 2 Oct 2023 11:40:36 +0200 Subject: [PATCH] Allow specifying the hashing method for CreateLookupVindex This change allows specifying the vschema fragment to override the hashing used for the backup table for the lookup vindex. This is something someone can manually already modify / set up in the vschema, but CreateLookupVindex does not support this properly. Signed-off-by: Dirkjan Bussink --- go.mod | 1 - go.sum | 2 - .../foreignkey/stress/fk_stress_test.go | 2 +- .../operators/subquery_planning.go | 3 +- go/vt/wrangler/materializer.go | 42 ++++-- go/vt/wrangler/materializer_test.go | 124 +++++++++++++++++- 6 files changed, 158 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 88e09e70d22..639a22edc6b 100644 --- a/go.mod +++ b/go.mod @@ -107,7 +107,6 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 github.com/xlab/treeprint v1.2.0 go.uber.org/goleak v1.2.1 - golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 golang.org/x/sync v0.3.0 modernc.org/sqlite v1.20.3 ) diff --git a/go.sum b/go.sum index cb54903b2e9..ffadd6498b9 100644 --- a/go.sum +++ b/go.sum @@ -677,8 +677,6 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= -golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 h1:m64FZMko/V45gv0bNmrNYoDEq8U5YUhetc9cBWKS1TQ= -golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63/go.mod h1:0v4NqG35kSWCMzLaMeX+IQrlSnVE/bqGSyC2cz/9Le8= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/go/test/endtoend/vtgate/foreignkey/stress/fk_stress_test.go b/go/test/endtoend/vtgate/foreignkey/stress/fk_stress_test.go index c55ce9eef79..8aa02bc6ca5 100644 --- a/go/test/endtoend/vtgate/foreignkey/stress/fk_stress_test.go +++ b/go/test/endtoend/vtgate/foreignkey/stress/fk_stress_test.go @@ -23,6 +23,7 @@ import ( "math/rand" "os" "path" + "slices" "strings" "sync" "testing" @@ -30,7 +31,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/exp/slices" "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/replication" diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index 93046648744..f9e78eb1abc 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -18,8 +18,7 @@ package operators import ( "io" - - "golang.org/x/exp/slices" + "slices" "vitess.io/vitess/go/slice" "vitess.io/vitess/go/vt/sqlparser" diff --git a/go/vt/wrangler/materializer.go b/go/vt/wrangler/materializer.go index 0fba424eacd..3915a2ba5b1 100644 --- a/go/vt/wrangler/materializer.go +++ b/go/vt/wrangler/materializer.go @@ -21,6 +21,7 @@ import ( "fmt" "hash/fnv" "math" + "slices" "sort" "strings" "sync" @@ -578,16 +579,33 @@ func (wr *Wrangler) prepareCreateLookup(ctx context.Context, keyspace string, sp } // Validate input table - if len(specs.Tables) != 1 { - return nil, nil, nil, fmt.Errorf("exactly one table must be specified in the specs: %v", specs.Tables) + if len(specs.Tables) < 1 || len(specs.Tables) > 2 { + return nil, nil, nil, fmt.Errorf("one or two tables must be specified in the specs: %v", specs.Tables) } - // Loop executes once. + + // Loop executes once or twice for k, ti := range specs.Tables { if len(ti.ColumnVindexes) != 1 { return nil, nil, nil, fmt.Errorf("exactly one ColumnVindex must be specified for the table: %v", specs.Tables) } - sourceTableName = k - sourceTable = ti + if k != targetTableName { + sourceTableName = k + sourceTable = ti + continue + } + + var vindexCols []string + if len(ti.ColumnVindexes[0].Columns) != 0 { + vindexCols = ti.ColumnVindexes[0].Columns + } else { + if ti.ColumnVindexes[0].Column == "" { + return nil, nil, nil, fmt.Errorf("at least one column must be specified in ColumnVindexes: %v", sourceTable.ColumnVindexes) + } + vindexCols = []string{ti.ColumnVindexes[0].Column} + } + if !slices.Equal(vindexCols, vindexFromCols) { + return nil, nil, nil, fmt.Errorf("columns in vindex don't match") + } } // Validate input table and vindex consistency @@ -741,16 +759,22 @@ func (wr *Wrangler) prepareCreateLookup(ctx context.Context, keyspace string, sp materializeQuery = buf.String() // Update targetVSchema - var targetTable *vschemapb.Table + targetTable := specs.Tables[targetTableName].CloneVT() + if targetVSchema.Sharded { // Choose a primary vindex type for target table based on source specs var targetVindexType string var targetVindex *vschemapb.Vindex for _, field := range tableSchema.TableDefinitions[0].Fields { if sourceVindexColumns[0] == field.Name { - targetVindexType, err = vindexes.ChooseVindexForType(field.Type) - if err != nil { - return nil, nil, nil, err + if targetTable != nil && len(targetTable.ColumnVindexes) > 0 { + targetVindexType = targetTable.ColumnVindexes[0].Name + } + if targetVindexType == "" { + targetVindexType, err = vindexes.ChooseVindexForType(field.Type) + if err != nil { + return nil, nil, nil, err + } } targetVindex = &vschemapb.Vindex{ Type: targetVindexType, diff --git a/go/vt/wrangler/materializer_test.go b/go/vt/wrangler/materializer_test.go index 9126d8540c1..cdcc79df68a 100644 --- a/go/vt/wrangler/materializer_test.go +++ b/go/vt/wrangler/materializer_test.go @@ -1333,6 +1333,128 @@ func TestCreateCustomizedVindex(t *testing.T) { } } +func TestCreateCustomizedHashVindex(t *testing.T) { + ms := &vtctldatapb.MaterializeSettings{ + SourceKeyspace: "ks", + TargetKeyspace: "ks", + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + env := newTestMaterializerEnv(t, ctx, ms, []string{"0"}, []string{"0"}) + defer env.close() + + specs := &vschemapb.Keyspace{ + Vindexes: map[string]*vschemapb.Vindex{ + "v": { + Type: "lookup_unique", + Params: map[string]string{ + "table": "ks.lkp", + "from": "c1", + "to": "col2", + }, + Owner: "t1", + }, + }, + Tables: map[string]*vschemapb.Table{ + "t1": { + ColumnVindexes: []*vschemapb.ColumnVindex{{ + Name: "v", + Column: "col2", + }}, + }, + "lkp": { + ColumnVindexes: []*vschemapb.ColumnVindex{{ + Name: "unicode_loose_xxhash", + Column: "c1", + }}, + }, + }, + } + // Dummy sourceSchema + sourceSchema := "CREATE TABLE `t1` (\n" + + " `col1` varchar(255) NOT NULL,\n" + + " `col2` varchar(255) DEFAULT NULL,\n" + + " PRIMARY KEY (`id`)\n" + + ") ENGINE=InnoDB" + + vschema := &vschemapb.Keyspace{ + Sharded: true, + Vindexes: map[string]*vschemapb.Vindex{ + "unicode_loose_md5": { + Type: "unicode_loose_md5", + }, + }, + Tables: map[string]*vschemapb.Table{ + "t1": { + ColumnVindexes: []*vschemapb.ColumnVindex{{ + Name: "unicode_loose_md5", + Column: "col1", + }}, + }, + }, + } + want := &vschemapb.Keyspace{ + Sharded: true, + Vindexes: map[string]*vschemapb.Vindex{ + "unicode_loose_md5": { + Type: "unicode_loose_md5", + }, + "unicode_loose_xxhash": { + Type: "unicode_loose_xxhash", + }, + "v": { + Type: "lookup_unique", + Params: map[string]string{ + "table": "ks.lkp", + "from": "c1", + "to": "col2", + "write_only": "true", + }, + Owner: "t1", + }, + }, + Tables: map[string]*vschemapb.Table{ + "t1": { + ColumnVindexes: []*vschemapb.ColumnVindex{{ + Name: "unicode_loose_md5", + Column: "col1", + }, { + Name: "v", + Column: "col2", + }}, + }, + "lkp": { + ColumnVindexes: []*vschemapb.ColumnVindex{{ + Column: "c1", + Name: "unicode_loose_xxhash", + }}, + }, + }, + } + env.tmc.schema[ms.SourceKeyspace+".t1"] = &tabletmanagerdatapb.SchemaDefinition{ + TableDefinitions: []*tabletmanagerdatapb.TableDefinition{{ + Fields: []*querypb.Field{{ + Name: "col1", + Type: querypb.Type_VARCHAR, + }, { + Name: "col2", + Type: querypb.Type_VARCHAR, + }}, + Schema: sourceSchema, + }}, + } + if err := env.topoServ.SaveVSchema(context.Background(), ms.SourceKeyspace, vschema); err != nil { + t.Fatal(err) + } + + _, got, _, err := env.wr.prepareCreateLookup(context.Background(), ms.SourceKeyspace, specs, false) + require.NoError(t, err) + if !proto.Equal(got, want) { + t.Errorf("customize create lookup error same: got:\n%v, want\n%v", got, want) + } +} + func TestCreateLookupVindexIgnoreNulls(t *testing.T) { ms := &vtctldatapb.MaterializeSettings{ SourceKeyspace: "ks", @@ -1673,7 +1795,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { input: &vschemapb.Keyspace{ Vindexes: unique, }, - err: "exactly one table must be specified in the specs", + err: "one or two tables must be specified in the specs", }, { description: "only one colvindex", input: &vschemapb.Keyspace{