Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-19.0] LookupVindex: fix CLI to allow creating non-unique lookups with single column (#17301) #17347

Draft
wants to merge 1 commit into
base: release-19.0
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions go/test/endtoend/vreplication/lookupindex_helper_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
204 changes: 204 additions & 0 deletions go/test/endtoend/vreplication/lookupindex_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
4 changes: 2 additions & 2 deletions go/vt/vtctl/workflow/materializer_test.go
Original file line number Diff line number Diff line change
@@ -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",
3 changes: 3 additions & 0 deletions go/vt/vtctl/workflow/server.go
Original file line number Diff line number Diff line change
@@ -3583,10 +3583,13 @@
if len(vindexFromCols) != 1 {
return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unique vindex 'from' should have only one column")
}
<<<<<<< HEAD

Check failure on line 3586 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: unexpected <<, expected }

Check failure on line 3586 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: unexpected <<, expected }
} else {

Check failure on line 3587 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: unexpected else after top level declaration

Check failure on line 3587 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: unexpected else after top level declaration
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))

Check failure on line 3592 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

invalid character U+0023 '#'

Check failure on line 3592 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

invalid character U+0023 '#'
}
vindexToCol = vindex.Params["to"]
// Make the vindex write_only. If one exists already in the vschema,
@@ -3626,7 +3629,7 @@
// This is a primary vindex definition for the target table
// which allows you to override the vindex type used.
var vindexCols []string
if len(table.ColumnVindexes[0].Columns) != 0 {

Check failure on line 3632 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: non-declaration statement outside function body

Check failure on line 3632 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: non-declaration statement outside function body
vindexCols = table.ColumnVindexes[0].Columns
} else {
if table.ColumnVindexes[0].Column == "" {
@@ -3735,7 +3738,7 @@
return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "schema looks incorrect: %s, expecting at least four lines", tableSchema.TableDefinitions[0].Schema)
}
var modified []string
modified = append(modified, strings.Replace(lines[0], sourceTableName, targetTableName, 1))

Check failure on line 3741 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: non-declaration statement outside function body

Check failure on line 3741 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: non-declaration statement outside function body
for i := range sourceVindexColumns {
line, err := generateColDef(lines, sourceVindexColumns[i], vindexFromCols[i])
if err != nil {
@@ -3800,7 +3803,7 @@
// definition if one was not explicitly specified.
var targetVindexType string
var targetVindex *vschemapb.Vindex
for _, field := range tableSchema.TableDefinitions[0].Fields {

Check failure on line 3806 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: non-declaration statement outside function body

Check failure on line 3806 in go/vt/vtctl/workflow/server.go

GitHub Actions / Code Coverage

syntax error: non-declaration statement outside function body
if sourceVindexColumns[0] == field.Name {
if targetTable != nil && len(targetTable.ColumnVindexes) > 0 {
targetVindexType = targetTable.ColumnVindexes[0].Name
5 changes: 1 addition & 4 deletions go/vt/wrangler/materializer.go
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions go/vt/wrangler/materializer_test.go
Original file line number Diff line number Diff line change
@@ -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{
Loading
Loading