From 7977edfec54ee162a098095f513263d0462bd928 Mon Sep 17 00:00:00 2001 From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com> Date: Mon, 9 Dec 2024 09:59:33 +0100 Subject: [PATCH] Cherry-pick 9a03317dcc7dabf722cb3b91a39c37809dcde720 with conflicts --- .../vreplication/lookupindex_helper_test.go | 113 ++++++++++ .../endtoend/vreplication/lookupindex_test.go | 204 ++++++++++++++++++ go/vt/vtctl/workflow/materializer_test.go | 4 +- go/vt/vtctl/workflow/server.go | 3 + go/vt/wrangler/materializer.go | 5 +- go/vt/wrangler/materializer_test.go | 4 +- test/config.json | 22 ++ 7 files changed, 347 insertions(+), 8 deletions(-) create mode 100644 go/test/endtoend/vreplication/lookupindex_helper_test.go create mode 100644 go/test/endtoend/vreplication/lookupindex_test.go diff --git a/go/test/endtoend/vreplication/lookupindex_helper_test.go b/go/test/endtoend/vreplication/lookupindex_helper_test.go new file mode 100644 index 00000000000..864a5e0f7fc --- /dev/null +++ b/go/test/endtoend/vreplication/lookupindex_helper_test.go @@ -0,0 +1,113 @@ +/* +Copyright 2024 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 vreplication + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + "vitess.io/vitess/go/vt/sqlparser" + + binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata" +) + +type lookupIndex struct { + typ string + name string + tableKeyspace string + table string + columns []string + ownerTable string + ownerTableKeyspace string + ignoreNulls bool + + t *testing.T +} + +func (li *lookupIndex) String() string { + return li.typ + " " + li.name + " on " + li.tableKeyspace + "." + li.table + " (" + li.columns[0] + ")" +} + +func (li *lookupIndex) create() { + cols := strings.Join(li.columns, ",") + args := []string{ + "LookupVindex", + "--name", li.name, + "--table-keyspace=" + li.ownerTableKeyspace, + "create", + "--keyspace=" + li.tableKeyspace, + "--type=" + li.typ, + "--table-owner=" + li.ownerTable, + "--table-owner-columns=" + cols, + "--tablet-types=PRIMARY", + } + if li.ignoreNulls { + args = append(args, "--ignore-nulls") + } + + err := vc.VtctldClient.ExecuteCommand(args...) + require.NoError(li.t, err, "error executing LookupVindex create: %v", err) + waitForWorkflowState(li.t, vc, fmt.Sprintf("%s.%s", li.ownerTableKeyspace, li.name), binlogdatapb.VReplicationWorkflowState_Running.String()) + li.expectWriteOnly(true) +} + +func (li *lookupIndex) cancel() { + panic("not implemented") +} + +func (li *lookupIndex) externalize() { + args := []string{ + "LookupVindex", + "--name", li.name, + "--table-keyspace=" + li.ownerTableKeyspace, + "externalize", + "--keyspace=" + li.tableKeyspace, + } + err := vc.VtctldClient.ExecuteCommand(args...) + require.NoError(li.t, err, "error executing LookupVindex externalize: %v", err) + li.expectWriteOnly(false) +} + +func (li *lookupIndex) show() error { + return nil +} + +func (li *lookupIndex) expectWriteOnly(expected bool) { + vschema, err := vc.VtctldClient.ExecuteCommandWithOutput("GetVSchema", li.ownerTableKeyspace) + require.NoError(li.t, err, "error executing GetVSchema: %v", err) + vdx := gjson.Get(vschema, fmt.Sprintf("vindexes.%s", li.name)) + require.NotNil(li.t, vdx, "lookup vindex %s not found", li.name) + want := "" + if expected { + want = "true" + } + require.Equal(li.t, want, vdx.Get("params.write_only").String(), "expected write_only parameter to be %s", want) +} + +func getNumRowsInQuery(t *testing.T, query string) int { + stmt, err := sqlparser.NewTestParser().Parse(query) + require.NoError(t, err) + insertStmt, ok := stmt.(*sqlparser.Insert) + require.True(t, ok) + rows, ok := insertStmt.Rows.(sqlparser.Values) + require.True(t, ok) + return len(rows) +} diff --git a/go/test/endtoend/vreplication/lookupindex_test.go b/go/test/endtoend/vreplication/lookupindex_test.go new file mode 100644 index 00000000000..348a0ee5906 --- /dev/null +++ b/go/test/endtoend/vreplication/lookupindex_test.go @@ -0,0 +1,204 @@ +/* +Copyright 2024 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 vreplication + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "vitess.io/vitess/go/test/endtoend/cluster" + vttablet "vitess.io/vitess/go/vt/vttablet/common" +) + +type TestClusterSpec struct { + keyspaceName string + vschema string + schema string +} + +var lookupClusterSpec = TestClusterSpec{ + keyspaceName: "lookup", + vschema: ` +{ + "sharded": true, + "vindexes": { + "reverse_bits": { + "type": "reverse_bits" + } + }, + "tables": { + "t1": { + "column_vindexes": [ + { + "column": "c1", + "name": "reverse_bits" + } + ] + } + } +} +`, + schema: ` +create table t1( + c1 int, + c2 int, + val varchar(128), + primary key(c1) +); +`, +} + +func setupLookupIndexKeyspace(t *testing.T) map[string]*cluster.VttabletProcess { + tablets := make(map[string]*cluster.VttabletProcess) + if _, err := vc.AddKeyspace(t, []*Cell{vc.Cells["zone1"]}, lookupClusterSpec.keyspaceName, "-80,80-", + lookupClusterSpec.vschema, lookupClusterSpec.schema, defaultReplicas, defaultRdonly, 200, nil); err != nil { + require.NoError(t, err) + } + defaultCell := vc.Cells[vc.CellNames[0]] + ks := vc.Cells[defaultCell.Name].Keyspaces[lookupClusterSpec.keyspaceName] + targetTab1 = ks.Shards["-80"].Tablets["zone1-200"].Vttablet + targetTab2 = ks.Shards["80-"].Tablets["zone1-300"].Vttablet + tablets["-80"] = targetTab1 + tablets["80-"] = targetTab2 + return tablets +} + +type lookupTestCase struct { + name string + li *lookupIndex + initQuery string + runningQuery string + postExternalizeQuery string + cleanupQuery string +} + +func TestLookupIndex(t *testing.T) { + setSidecarDBName("_vt") + origDefaultReplicas := defaultReplicas + origDefaultRdonly := defaultRdonly + defer func() { + defaultReplicas = origDefaultReplicas + defaultRdonly = origDefaultRdonly + }() + defaultReplicas = 1 + defaultRdonly = 0 + vc = setupMinimalCluster(t) + defer vc.TearDown() + vttablet.InitVReplicationConfigDefaults() + + _ = setupLookupIndexKeyspace(t) + + initQuery := "insert into t1 (c1, c2, val) values (1, 1, 'val1'), (2, 2, 'val2'), (3, 3, 'val3')" + runningQuery := "insert into t1 (c1, c2, val) values (4, 4, 'val4'), (5, 5, 'val5'), (6, 6, 'val6')" + postExternalizeQuery := "insert into t1 (c1, c2, val) values (7, 7, 'val7'), (8, 8, 'val8'), (9, 9, 'val9')" + cleanupQuery := "delete from t1" + + testCases := []lookupTestCase{ + { + name: "non-unique lookup index, one column", + li: &lookupIndex{ + typ: "consistent_lookup", + name: "t1_c2_lookup", + tableKeyspace: lookupClusterSpec.keyspaceName, + table: "t1", + columns: []string{"c2"}, + ownerTable: "t1", + ownerTableKeyspace: lookupClusterSpec.keyspaceName, + ignoreNulls: true, + t: t, + }, + }, + { + name: "lookup index, two columns", + li: &lookupIndex{ + typ: "lookup", + name: "t1_c2_val_lookup", + tableKeyspace: lookupClusterSpec.keyspaceName, + table: "t1", + columns: []string{"c2", "val"}, + ownerTable: "t1", + ownerTableKeyspace: lookupClusterSpec.keyspaceName, + ignoreNulls: true, + t: t, + }, + }, + { + name: "unique lookup index, one column", + li: &lookupIndex{ + typ: "lookup_unique", + name: "t1_c2_unique_lookup", + tableKeyspace: lookupClusterSpec.keyspaceName, + table: "t1", + columns: []string{"c2"}, + ownerTable: "t1", + ownerTableKeyspace: lookupClusterSpec.keyspaceName, + ignoreNulls: true, + t: t, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.initQuery = initQuery + tc.runningQuery = runningQuery + tc.postExternalizeQuery = postExternalizeQuery + tc.cleanupQuery = cleanupQuery + testLookupVindex(t, &tc) + }) + } +} + +func testLookupVindex(t *testing.T, tc *lookupTestCase) { + vtgateConn, cancel := getVTGateConn() + defer cancel() + var totalRows int + li := tc.li + + t.Run("init data", func(t *testing.T) { + totalRows += getNumRowsInQuery(t, tc.initQuery) + _, err := vtgateConn.ExecuteFetch(tc.initQuery, 1000, false) + require.NoError(t, err) + }) + + t.Run("create", func(t *testing.T) { + tc.li.create() + + lks := li.tableKeyspace + vindexName := li.name + waitForRowCount(t, vtgateConn, lks, vindexName, totalRows) + totalRows += getNumRowsInQuery(t, tc.runningQuery) + _, err := vtgateConn.ExecuteFetch(tc.runningQuery, 1000, false) + require.NoError(t, err) + waitForRowCount(t, vtgateConn, tc.li.ownerTableKeyspace, li.name, totalRows) + }) + + t.Run("externalize", func(t *testing.T) { + tc.li.externalize() + totalRows += getNumRowsInQuery(t, tc.postExternalizeQuery) + _, err := vtgateConn.ExecuteFetch(tc.postExternalizeQuery, 1000, false) + require.NoError(t, err) + waitForRowCount(t, vtgateConn, tc.li.ownerTableKeyspace, li.name, totalRows) + }) + + t.Run("cleanup", func(t *testing.T) { + _, err := vtgateConn.ExecuteFetch(tc.cleanupQuery, 1000, false) + require.NoError(t, err) + waitForRowCount(t, vtgateConn, tc.li.ownerTableKeyspace, li.name, 0) + }) +} diff --git a/go/vt/vtctl/workflow/materializer_test.go b/go/vt/vtctl/workflow/materializer_test.go index e403ab8b4d5..8cf2d2f819c 100644 --- a/go/vt/vtctl/workflow/materializer_test.go +++ b/go/vt/vtctl/workflow/materializer_test.go @@ -1976,7 +1976,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { err: "unique vindex 'from' should have only one column", }, { - description: "non-unique lookup should have more than one column", + description: "non-unique lookup can have only one column", input: &vschemapb.Keyspace{ Vindexes: map[string]*vschemapb.Vindex{ "v": { @@ -1989,7 +1989,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { }, }, }, - err: "non-unique vindex 'from' should have more than one column", + err: "", }, { description: "vindex not found", diff --git a/go/vt/vtctl/workflow/server.go b/go/vt/vtctl/workflow/server.go index c9a512e4bf5..9dd929940db 100644 --- a/go/vt/vtctl/workflow/server.go +++ b/go/vt/vtctl/workflow/server.go @@ -3583,10 +3583,13 @@ func (s *Server) prepareCreateLookup(ctx context.Context, workflow, keyspace str if len(vindexFromCols) != 1 { return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unique vindex 'from' should have only one column") } +<<<<<<< HEAD } else { if len(vindexFromCols) < 2 { return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "non-unique vindex 'from' should have more than one column") } +======= +>>>>>>> 9a03317dcc (LookupVindex: fix CLI to allow creating non-unique lookups with single column (#17301)) } vindexToCol = vindex.Params["to"] // Make the vindex write_only. If one exists already in the vschema, diff --git a/go/vt/wrangler/materializer.go b/go/vt/wrangler/materializer.go index 5b159e35a11..d69065fe69d 100644 --- a/go/vt/wrangler/materializer.go +++ b/go/vt/wrangler/materializer.go @@ -550,11 +550,8 @@ func (wr *Wrangler) prepareCreateLookup(ctx context.Context, keyspace string, sp if len(vindexFromCols) != 1 { return nil, nil, nil, fmt.Errorf("unique vindex 'from' should have only one column: %v", vindex) } - } else { - if len(vindexFromCols) < 2 { - return nil, nil, nil, fmt.Errorf("non-unique vindex 'from' should have more than one column: %v", vindex) - } } + vindexToCol = vindex.Params["to"] // Make the vindex write_only. If one exists already in the vschema, // it will need to match this vindex exactly, including the write_only setting. diff --git a/go/vt/wrangler/materializer_test.go b/go/vt/wrangler/materializer_test.go index bdeee1e6ac3..1d08c986133 100644 --- a/go/vt/wrangler/materializer_test.go +++ b/go/vt/wrangler/materializer_test.go @@ -1644,7 +1644,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { }, err: "unique vindex 'from' should have only one column", }, { - description: "non-unique lookup should have more than one column", + description: "non-unique lookup can have only one column", input: &vschemapb.Keyspace{ Vindexes: map[string]*vschemapb.Vindex{ "v": { @@ -1657,7 +1657,7 @@ func TestCreateLookupVindexFailures(t *testing.T) { }, }, }, - err: "non-unique vindex 'from' should have more than one column", + err: "", }, { description: "vindex not found", input: &vschemapb.Keyspace{ diff --git a/test/config.json b/test/config.json index d21994ddca0..7a5fa92976e 100644 --- a/test/config.json +++ b/test/config.json @@ -1288,7 +1288,29 @@ "Args": ["vitess.io/vitess/go/test/endtoend/vreplication", "-run", "TestMoveTablesTZ"], "Command": [], "Manual": false, +<<<<<<< HEAD "Shard": "vreplication_cli_migrate_vdiff2_convert_tz", +======= + "Shard": "vreplication_vtctldclient_vdiff2_movetables_tz", + "RetryMax": 1, + "Tags": [] + }, + "loopkup_index": { + "File": "unused.go", + "Args": ["vitess.io/vitess/go/test/endtoend/vreplication", "-run", "TestLookupIndex"], + "Command": [], + "Manual": false, + "Shard": "vreplication_vtctldclient_vdiff2_movetables_tz", + "RetryMax": 1, + "Tags": [] + }, + "vtadmin": { + "File": "unused.go", + "Args": ["vitess.io/vitess/go/test/endtoend/vtadmin"], + "Command": [], + "Manual": false, + "Shard": "15", +>>>>>>> 9a03317dcc (LookupVindex: fix CLI to allow creating non-unique lookups with single column (#17301)) "RetryMax": 1, "Tags": [] },