From c4c213b2645c0ce3771b86bd5e58210bbfe1f170 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 24 Jan 2023 14:50:15 +0200 Subject: [PATCH 01/34] validate table columns referenced by views Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/diff_test.go | 1 + go/vt/schemadiff/errors.go | 20 ++++ go/vt/schemadiff/schema.go | 178 ++++++++++++++++++++++++++++++++ go/vt/schemadiff/schema_test.go | 8 +- 4 files changed, 203 insertions(+), 4 deletions(-) diff --git a/go/vt/schemadiff/diff_test.go b/go/vt/schemadiff/diff_test.go index 668895a7891..db45fb789b8 100644 --- a/go/vt/schemadiff/diff_test.go +++ b/go/vt/schemadiff/diff_test.go @@ -667,6 +667,7 @@ func TestDiffSchemas(t *testing.T) { // validate schema1 unaffected by Apply assert.Equal(t, schema1SQL, schema1.ToSQL()) + require.NotNil(t, schema2) appliedDiff, err := schema2.Diff(applied, hints) require.NoError(t, err) assert.Empty(t, appliedDiff) diff --git a/go/vt/schemadiff/errors.go b/go/vt/schemadiff/errors.go index 54130445044..d326b961cc6 100644 --- a/go/vt/schemadiff/errors.go +++ b/go/vt/schemadiff/errors.go @@ -308,3 +308,23 @@ type ViewDependencyUnresolvedError struct { func (e *ViewDependencyUnresolvedError) Error() string { return fmt.Sprintf("view %s has unresolved/loop dependencies", sqlescape.EscapeID(e.View)) } + +type InvalidColumnReferencedInViewError struct { + View string + Table string + Column string + NonUnique bool +} + +func (e *InvalidColumnReferencedInViewError) Error() string { + switch { + case e.Column == "": + return fmt.Sprintf("view %s references non-existing table %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table)) + case e.Table != "": + return fmt.Sprintf("view %s references non existing column %s.%s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table), sqlescape.EscapeID(e.Column)) + case e.NonUnique: + return fmt.Sprintf("view %s references unqualified but non unique column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) + default: + return fmt.Sprintf("view %s references unqualified but non existing column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) + } +} diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 939abc47960..0872cf88a5e 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -24,6 +24,8 @@ import ( "sort" "strings" + "go.uber.org/multierr" + "vitess.io/vitess/go/vt/sqlparser" ) @@ -309,6 +311,11 @@ func (s *Schema) normalize() error { } } + // Validate views' referenced columns: do these columns actually exist in referenced tables/views? + if err := s.ValidateViewReferences(); err != nil { + return err + } + // Validate table definitions for _, t := range s.tables { if err := t.validate(); err != nil { @@ -750,3 +757,174 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { } return dup, nil } + +// TODO + +func (s *Schema) ValidateViewReferences() error { + var allerrors error + availableColumns := map[string]map[string]struct{}{} + for _, table := range s.Tables() { + availableColumns[table.Name()] = map[string]struct{}{} + for _, col := range table.TableSpec.Columns { + availableColumns[table.Name()][col.Name.Lowered()] = struct{}{} + } + } + // Add dual table with no explicit columns for dual style expressions in views. + availableColumns["dual"] = map[string]struct{}{} + + for _, view := range s.Views() { + // First gather all referenced tables and table aliases + tableAliases := map[string]string{} + tableReferences := map[string]struct{}{} + err := gatherTableInformationForView(view, availableColumns, tableReferences, tableAliases) + allerrors = multierr.Append(allerrors, err) + + // Now we can walk the view again and check each column expression + // to see if there's an existing column referenced. + err = gatherColumnReferenceInformationForView(view, availableColumns, tableReferences, tableAliases) + allerrors = multierr.Append(allerrors, err) + } + return allerrors +} + +func gatherTableInformationForView(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableReferences map[string]struct{}, tableAliases map[string]string) error { + var allerrors error + tableErrors := make(map[string]struct{}) + err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + switch node := node.(type) { + case *sqlparser.AliasedTableExpr: + aliased := sqlparser.GetTableName(node.Expr).String() + if aliased == "" { + return true, nil + } + + if _, ok := availableColumns[aliased]; !ok { + if _, ok := tableErrors[aliased]; ok { + // Only show a missing table reference once per view. + return true, nil + } + err := &InvalidColumnReferencedInViewError{ + View: view.Name(), + Table: aliased, + } + allerrors = multierr.Append(allerrors, err) + tableErrors[aliased] = struct{}{} + return true, nil + } + tableReferences[aliased] = struct{}{} + if node.As.String() != "" { + tableAliases[node.As.String()] = aliased + } + } + return true, nil + }, view.Select) + if err != nil { + // parsing error. Forget about any view dependency issues we may have found. This is way more important + return err + } + return allerrors +} + +func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableReferences map[string]struct{}, tableAliases map[string]string) error { + var allerrors error + qualifiedColumnErrors := make(map[string]map[string]struct{}) + unqualifiedColumnErrors := make(map[string]struct{}) + + err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + switch node := node.(type) { + case *sqlparser.ColName: + if node.Qualifier.IsEmpty() { + err := verifyUnqualifiedColumn(view, availableColumns, tableReferences, node.Name, unqualifiedColumnErrors) + allerrors = multierr.Append(allerrors, err) + } else { + err := verifyQualifiedColumn(view, availableColumns, tableAliases, node, qualifiedColumnErrors) + allerrors = multierr.Append(allerrors, err) + } + } + return true, nil + }, view.Select) + if err != nil { + // parsing error. Forget about any view dependency issues we may have found. This is way more important + return err + } + return allerrors +} + +func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { + // In case we have a non-qualified column reference, it needs + // to be unique across all referenced tables if it is supposed + // to work. + columnFound := false + for table := range tableReferences { + cols, ok := availableColumns[table] + if !ok { + // We already dealt with an error for a missing table reference + // earlier, so we can ignore it at this point here. + return nil + } + _, columnInTable := cols[nodeName.Lowered()] + if !columnInTable { + continue + } + if columnFound { + // We already have seen the column before in another table, so + // if we see it again here, that's an error case. + if _, ok := unqualifiedColumnErrors[nodeName.Lowered()]; ok { + return nil + } + unqualifiedColumnErrors[nodeName.Lowered()] = struct{}{} + return &InvalidColumnReferencedInViewError{ + View: view.Name(), + Column: nodeName.String(), + NonUnique: true, + } + } + columnFound = true + } + + // If we've seen the desired column here once, we're all good + if columnFound { + return nil + } + + if _, ok := unqualifiedColumnErrors[nodeName.Lowered()]; ok { + return nil + } + unqualifiedColumnErrors[nodeName.Lowered()] = struct{}{} + return &InvalidColumnReferencedInViewError{ + View: view.Name(), + Column: nodeName.String(), + } +} + +func verifyQualifiedColumn(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableAliases map[string]string, node *sqlparser.ColName, columnErrors map[string]map[string]struct{}) error { + tableName := node.Qualifier.Name.String() + if aliased, ok := tableAliases[tableName]; ok { + tableName = aliased + } + cols, ok := availableColumns[tableName] + if !ok { + // Already dealt with missing tables earlier on, we don't have + // any error to add here. + return nil + } + _, ok = cols[node.Name.Lowered()] + if ok { + // Found the column in the table, all good. + return nil + } + + if _, ok := columnErrors[tableName]; !ok { + columnErrors[tableName] = make(map[string]struct{}) + } + + if _, ok := columnErrors[tableName][node.Name.Lowered()]; ok { + return nil + } + columnErrors[tableName][node.Name.Lowered()] = struct{}{} + return &InvalidColumnReferencedInViewError{ + View: view.Name(), + Table: tableName, + Column: node.Name.String(), + } +} diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index cd2baa8527a..49ee23172ea 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -79,7 +79,7 @@ var toSQL = "CREATE TABLE `t1` (\n\t`id` int\n);\nCREATE TABLE `t2` (\n\t`id` in func TestNewSchemaFromQueries(t *testing.T) { schema, err := NewSchemaFromQueries(createQueries) assert.NoError(t, err) - assert.NotNil(t, schema) + require.NotNil(t, schema) assert.Equal(t, expectSortedNames, schema.EntityNames()) assert.Equal(t, expectSortedTableNames, schema.TableNames()) @@ -89,7 +89,7 @@ func TestNewSchemaFromQueries(t *testing.T) { func TestNewSchemaFromSQL(t *testing.T) { schema, err := NewSchemaFromSQL(strings.Join(createQueries, ";")) assert.NoError(t, err) - assert.NotNil(t, schema) + require.NotNil(t, schema) assert.Equal(t, expectSortedNames, schema.EntityNames()) assert.Equal(t, expectSortedTableNames, schema.TableNames()) @@ -140,7 +140,7 @@ func TestNewSchemaFromQueriesLoop(t *testing.T) { func TestToSQL(t *testing.T) { schema, err := NewSchemaFromQueries(createQueries) assert.NoError(t, err) - assert.NotNil(t, schema) + require.NotNil(t, schema) sql := schema.ToSQL() assert.Equal(t, toSQL, sql) @@ -149,7 +149,7 @@ func TestToSQL(t *testing.T) { func TestCopy(t *testing.T) { schema, err := NewSchemaFromQueries(createQueries) assert.NoError(t, err) - assert.NotNil(t, schema) + require.NotNil(t, schema) schemaClone := schema.copy() assert.Equal(t, schema, schemaClone) From 3dac9070ca529bae1dc7c03be858870c96264979 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 24 Jan 2023 17:49:12 +0200 Subject: [PATCH 02/34] validate views' columns Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/errors.go | 8 ++ go/vt/schemadiff/schema.go | 62 ++++++++- go/vt/schemadiff/schema_test.go | 233 +++++++++++++++++++++++++++++++- 3 files changed, 297 insertions(+), 6 deletions(-) diff --git a/go/vt/schemadiff/errors.go b/go/vt/schemadiff/errors.go index d326b961cc6..9dfc2ff81be 100644 --- a/go/vt/schemadiff/errors.go +++ b/go/vt/schemadiff/errors.go @@ -328,3 +328,11 @@ func (e *InvalidColumnReferencedInViewError) Error() string { return fmt.Sprintf("view %s references unqualified but non existing column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) } } + +type EntityNotFoundError struct { + Name string +} + +func (e *EntityNotFoundError) Error() string { + return fmt.Sprintf("entity %s not found", sqlescape.EscapeID(e.Name)) +} diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 0872cf88a5e..18a29500084 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -763,12 +763,18 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { func (s *Schema) ValidateViewReferences() error { var allerrors error availableColumns := map[string]map[string]struct{}{} - for _, table := range s.Tables() { - availableColumns[table.Name()] = map[string]struct{}{} - for _, col := range table.TableSpec.Columns { - availableColumns[table.Name()][col.Name.Lowered()] = struct{}{} + + for _, e := range s.Entities() { + entityColumns, err := s.getEntityColumnNames(e.Name()) + if err != nil { + return err + } + availableColumns[e.Name()] = map[string]struct{}{} + for _, col := range entityColumns { + availableColumns[e.Name()][col.Lowered()] = struct{}{} } } + // Add dual table with no explicit columns for dual style expressions in views. availableColumns["dual"] = map[string]struct{}{} @@ -928,3 +934,51 @@ func verifyQualifiedColumn(view *CreateViewEntity, availableColumns map[string]m Column: node.Name.String(), } } + +// getTableColumnNames returns the names of columns in given table. +func (s *Schema) getEntityColumnNames(entityName string) (columnNames []*sqlparser.IdentifierCI, err error) { + entity := s.Entity(entityName) + if entity == nil { + if strings.ToLower(entityName) == "dual" { + // this is fine. DUAL does not exist but is allowed + return nil, nil + } + return nil, &EntityNotFoundError{Name: entityName} + } + // The entity is either a table or a view + switch entity := entity.(type) { + case *CreateTableEntity: + return s.getTableColumnNames(entity), nil + case *CreateViewEntity: + return s.getViewColumnNames(entity) + } + return nil, &EntityNotFoundError{Name: entityName} +} + +// getTableColumnNames returns the names of columns in given table. +func (s *Schema) getTableColumnNames(t *CreateTableEntity) (columnNames []*sqlparser.IdentifierCI) { + for _, c := range t.TableSpec.Columns { + columnNames = append(columnNames, &c.Name) + } + return columnNames +} + +// getViewColumnNames returns the names of aliased columns returned by a given view. +func (s *Schema) getViewColumnNames(v *CreateViewEntity) (columnNames []*sqlparser.IdentifierCI, err error) { + err = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + switch node := node.(type) { + case *sqlparser.AliasedExpr: + if node.As.String() != "" { + columnNames = append(columnNames, &node.As) + } else { + name := sqlparser.NewIdentifierCI(sqlparser.String(node.Expr)) + columnNames = append(columnNames, &name) + } + } + return true, nil + }, v.Select.GetColumns()) + if err != nil { + return nil, err + } + return columnNames, nil +} diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index 49ee23172ea..78308255d40 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -36,7 +36,7 @@ var createQueries = []string{ "create table t5(id int)", "create view v2 as select * from v3, t2", "create view v1 as select * from v3", - "create view v3 as select * from t3 as t3", + "create view v3 as select *, id+1 as id_plus, id+2 from t3 as t3", "create view v0 as select 1 from DUAL", "create view v9 as select 1", } @@ -74,7 +74,7 @@ var expectSortedViewNames = []string{ "v6", // level 3 } -var toSQL = "CREATE TABLE `t1` (\n\t`id` int\n);\nCREATE TABLE `t2` (\n\t`id` int\n);\nCREATE TABLE `t3` (\n\t`id` int,\n\t`type` enum('foo', 'bar') NOT NULL DEFAULT 'foo'\n);\nCREATE TABLE `t5` (\n\t`id` int\n);\nCREATE VIEW `v0` AS SELECT 1 FROM `dual`;\nCREATE VIEW `v3` AS SELECT * FROM `t3` AS `t3`;\nCREATE VIEW `v9` AS SELECT 1 FROM `dual`;\nCREATE VIEW `v1` AS SELECT * FROM `v3`;\nCREATE VIEW `v2` AS SELECT * FROM `v3`, `t2`;\nCREATE VIEW `v4` AS SELECT * FROM `t2` AS `something_else`, `v3`;\nCREATE VIEW `v5` AS SELECT * FROM `t1`, (SELECT * FROM `v3`) AS `some_alias`;\nCREATE VIEW `v6` AS SELECT * FROM `v4`;\n" +var toSQL = "CREATE TABLE `t1` (\n\t`id` int\n);\nCREATE TABLE `t2` (\n\t`id` int\n);\nCREATE TABLE `t3` (\n\t`id` int,\n\t`type` enum('foo', 'bar') NOT NULL DEFAULT 'foo'\n);\nCREATE TABLE `t5` (\n\t`id` int\n);\nCREATE VIEW `v0` AS SELECT 1 FROM `dual`;\nCREATE VIEW `v3` AS SELECT *, `id` + 1 AS `id_plus`, `id` + 2 FROM `t3` AS `t3`;\nCREATE VIEW `v9` AS SELECT 1 FROM `dual`;\nCREATE VIEW `v1` AS SELECT * FROM `v3`;\nCREATE VIEW `v2` AS SELECT * FROM `v3`, `t2`;\nCREATE VIEW `v4` AS SELECT * FROM `t2` AS `something_else`, `v3`;\nCREATE VIEW `v5` AS SELECT * FROM `t1`, (SELECT * FROM `v3`) AS `some_alias`;\nCREATE VIEW `v6` AS SELECT * FROM `v4`;\n" func TestNewSchemaFromQueries(t *testing.T) { schema, err := NewSchemaFromQueries(createQueries) @@ -376,3 +376,232 @@ func TestInvalidTableForeignKeyReference(t *testing.T) { assert.EqualError(t, err, (&ForeignKeyDependencyUnresolvedError{Table: "t11"}).Error()) } } + +func TestGetEntityColumnNames(t *testing.T) { + var queries = []string{ + "create table t1(id int, state int, some char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select id as id from t1", + "create view v2 as select 3+1 as `id`, state as state, some as thing from t1", + "create view v3 as select `id` as `id`, state as state, thing as another from v2", + "create view v4 as select 1 as `ok` from dual", + "create view v5 as select 1 as `ok` from DUAL", + "create view v6 as select ok as `ok` from v5", + } + + schema, err := NewSchemaFromQueries(queries) + require.NoError(t, err) + require.NotNil(t, schema) + + expectedColNames := map[string]([]string){ + "t1": []string{"id", "state", "some"}, + "t2": []string{"id", "c"}, + "v1": []string{"id"}, + "v2": []string{"id", "state", "thing"}, + "v3": []string{"id", "state", "another"}, + "v4": []string{"ok"}, + "v5": []string{"ok"}, + "v6": []string{"ok"}, + } + for tbl, expectNames := range expectedColNames { + identifiers, err := schema.getEntityColumnNames(tbl) + assert.NoError(t, err) + names := []string{} + for _, ident := range identifiers { + names = append(names, ident.String()) + } + assert.Equal(t, expectNames, names) + } +} + +func TestViewReferences(t *testing.T) { + tt := []struct { + name string + queries []string + expectErr error + }{ + { + name: "valid", + queries: []string{ + "create table t1(id int, state int, some char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select id as id from t1", + "create view v2 as select 3+1 as `id`, state as state, some as thing from t1", + "create view v3 as select `id` as `id`, state as state, thing as another from v2", + "create view v4 as select 1 as `ok` from dual", + "create view v5 as select 1 as `ok` from DUAL", + "create view v6 as select ok as `ok` from v5", + }, + }, + { + name: "valid WHERE", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select c from t1 where id=3", + }, + }, + { + name: "invalid unqualified referenced column", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select unexpected from t1", + }, + expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "unexpected"}, + }, + { + name: "invalid unqualified referenced column in where clause", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select 1 from t1 where unexpected=3", + }, + expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "unexpected"}, + }, + { + name: "valid qualified", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select t1.c from t1 where t1.id=3", + }, + }, + { + name: "valid qualified, multiple tables", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select t1.c from t1, t2 where t2.id=3", + }, + }, + { + name: "invalid unqualified, multiple tables", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select c from t1, t2 where t2.id=3", + }, + expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "c", NonUnique: true}, + }, + { + name: "invalid unqualified in WHERE clause, multiple tables", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5))", + "create view v1 as select t2.c from t1, t2 where id=3", + }, + expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "id", NonUnique: true}, + }, + { + name: "valid unqualified, multiple tables", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5), only_in_t2 int)", + "create view v1 as select only_in_t2 from t1, t2 where t1.id=3", + }, + }, + { + name: "valid unqualified in WHERE clause, multiple tables", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, c char(5), only_in_t2 int)", + "create view v1 as select t1.id from t1, t2 where only_in_t2=3", + }, + }, + { + name: "valid cascaded views", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id, c from t1 where id > 0", + "create view v2 as select * from v1 where id > 0", + }, + }, + { + name: "valid cascaded views, column aliases", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id, c as ch from t1 where id > 0", + "create view v2 as select ch from v1 where id > 0", + }, + }, + { + name: "valid cascaded views, column aliases in WHERE", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id as counter, c as ch from t1 where id > 0", + "create view v2 as select ch from v1 where counter > 0", + }, + }, + { + name: "valid cascaded views, aliased expression", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id+1 as counter, c as ch from t1 where id > 0", + "create view v2 as select ch from v1 where counter > 0", + }, + }, + { + name: "valid cascaded views, non aliased expression", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id+1, c as ch from t1 where id > 0", + "create view v2 as select ch from v1 where `id + 1` > 0", + }, + }, + { + name: "cascaded views, invalid column aliases", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id, c as ch from t1 where id > 0", + "create view v2 as select c from v1 where id > 0", + }, + expectErr: &InvalidColumnReferencedInViewError{View: "v2", Column: "c"}, + }, + { + name: "cascaded views, column not in view", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id from t1 where c='x'", + "create view v2 as select c from v1 where id > 0", + }, + expectErr: &InvalidColumnReferencedInViewError{View: "v2", Column: "c"}, + }, + { + name: "complex cascade", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, n int, info int)", + "create view v1 as select id, c as ch from t1 where id > 0", + "create view v2 as select n as num, info from t2", + "create view v3 as select num, v1.id, ch from v1 join v2 using (id) where info > 5", + }, + }, + { + name: "valid *", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select * from t1 where id > 0", + }, + }, + { + name: "valid *, cascaded", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select t1.* from t1 where id > 0", + "create view v2 as select * from v1 where id > 0", + }, + }, + } + for _, ts := range tt { + t.Run(ts.name, func(t *testing.T) { + schema, err := NewSchemaFromQueries(ts.queries) + if ts.expectErr == nil { + require.NoError(t, err) + require.NotNil(t, schema) + } else { + require.Equal(t, ts.expectErr, err, "received error: %v", err) + } + }) + } +} From e88d426d11eac540db3a3cda7268fd9bd0e0dd16 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 25 Jan 2023 11:35:23 +0200 Subject: [PATCH 03/34] support star expression Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 50 ++++++++++++++++++---- go/vt/schemadiff/schema_test.go | 75 ++++++++++++++++++++++++++++----- 2 files changed, 106 insertions(+), 19 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 18a29500084..deb127df811 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -29,6 +29,8 @@ import ( "vitess.io/vitess/go/vt/sqlparser" ) +type tablesColumnsMap map[string]map[string]struct{} + // Schema represents a database schema, which may contain entities such as tables and views. // Schema is not in itself an Entity, since it is more of a collection of entities. type Schema struct { @@ -762,10 +764,10 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { func (s *Schema) ValidateViewReferences() error { var allerrors error - availableColumns := map[string]map[string]struct{}{} + availableColumns := tablesColumnsMap{} for _, e := range s.Entities() { - entityColumns, err := s.getEntityColumnNames(e.Name()) + entityColumns, err := s.getEntityColumnNames(e.Name(), availableColumns) if err != nil { return err } @@ -793,7 +795,7 @@ func (s *Schema) ValidateViewReferences() error { return allerrors } -func gatherTableInformationForView(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableReferences map[string]struct{}, tableAliases map[string]string) error { +func gatherTableInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { var allerrors error tableErrors := make(map[string]struct{}) err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { @@ -831,7 +833,7 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns map[ return allerrors } -func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableReferences map[string]struct{}, tableAliases map[string]string) error { +func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { var allerrors error qualifiedColumnErrors := make(map[string]map[string]struct{}) unqualifiedColumnErrors := make(map[string]struct{}) @@ -856,7 +858,7 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo return allerrors } -func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { +func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { // In case we have a non-qualified column reference, it needs // to be unique across all referenced tables if it is supposed // to work. @@ -903,7 +905,12 @@ func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns map[string } } -func verifyQualifiedColumn(view *CreateViewEntity, availableColumns map[string]map[string]struct{}, tableAliases map[string]string, node *sqlparser.ColName, columnErrors map[string]map[string]struct{}) error { +func verifyQualifiedColumn( + view *CreateViewEntity, + availableColumns tablesColumnsMap, + tableAliases map[string]string, node *sqlparser.ColName, + columnErrors map[string]map[string]struct{}, +) error { tableName := node.Qualifier.Name.String() if aliased, ok := tableAliases[tableName]; ok { tableName = aliased @@ -936,7 +943,10 @@ func verifyQualifiedColumn(view *CreateViewEntity, availableColumns map[string]m } // getTableColumnNames returns the names of columns in given table. -func (s *Schema) getEntityColumnNames(entityName string) (columnNames []*sqlparser.IdentifierCI, err error) { +func (s *Schema) getEntityColumnNames(entityName string, availableColumns tablesColumnsMap) ( + columnNames []*sqlparser.IdentifierCI, + err error, +) { entity := s.Entity(entityName) if entity == nil { if strings.ToLower(entityName) == "dual" { @@ -950,7 +960,7 @@ func (s *Schema) getEntityColumnNames(entityName string) (columnNames []*sqlpars case *CreateTableEntity: return s.getTableColumnNames(entity), nil case *CreateViewEntity: - return s.getViewColumnNames(entity) + return s.getViewColumnNames(entity, availableColumns) } return nil, &EntityNotFoundError{Name: entityName} } @@ -964,9 +974,31 @@ func (s *Schema) getTableColumnNames(t *CreateTableEntity) (columnNames []*sqlpa } // getViewColumnNames returns the names of aliased columns returned by a given view. -func (s *Schema) getViewColumnNames(v *CreateViewEntity) (columnNames []*sqlparser.IdentifierCI, err error) { +func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns tablesColumnsMap) ( + columnNames []*sqlparser.IdentifierCI, + err error, +) { err = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { switch node := node.(type) { + case *sqlparser.StarExpr: + if tableName := node.TableName.Name.String(); tableName != "" { + for colName := range availableColumns[tableName] { + name := sqlparser.NewIdentifierCI(colName) + columnNames = append(columnNames, &name) + } + } else { + dependentNames, err := getViewDependentTableNames(v.CreateView) + if err != nil { + return false, err + } + // add all columns from all referenced tables and views + for _, entityName := range dependentNames { + for colName := range availableColumns[entityName] { + name := sqlparser.NewIdentifierCI(colName) + columnNames = append(columnNames, &name) + } + } + } case *sqlparser.AliasedExpr: if node.As.String() != "" { columnNames = append(columnNames, &node.As) diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index 78308255d40..c24b1afd20b 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -17,6 +17,7 @@ limitations under the License. package schemadiff import ( + "sort" "strings" "testing" @@ -387,6 +388,11 @@ func TestGetEntityColumnNames(t *testing.T) { "create view v4 as select 1 as `ok` from dual", "create view v5 as select 1 as `ok` from DUAL", "create view v6 as select ok as `ok` from v5", + "create view v7 as select * from t1", + "create view v8 as select * from v7", + "create view v9 as select * from v8, v6", + "create view va as select * from v6, v8", + "create view vb as select *, now() from v8", } schema, err := NewSchemaFromQueries(queries) @@ -402,15 +408,37 @@ func TestGetEntityColumnNames(t *testing.T) { "v4": []string{"ok"}, "v5": []string{"ok"}, "v6": []string{"ok"}, + "v7": []string{"id", "state", "some"}, + "v8": []string{"id", "state", "some"}, + "v9": []string{"id", "state", "some", "ok"}, + "va": []string{"ok", "id", "state", "some"}, + "vb": []string{"id", "state", "some", "now()"}, } - for tbl, expectNames := range expectedColNames { - identifiers, err := schema.getEntityColumnNames(tbl) - assert.NoError(t, err) - names := []string{} - for _, ident := range identifiers { - names = append(names, ident.String()) - } - assert.Equal(t, expectNames, names) + entities := schema.Entities() + require.Equal(t, len(entities), len(expectedColNames)) + + tcmap := tablesColumnsMap{} + // we test by order of dependency: + for _, e := range entities { + tbl := e.Name() + t.Run(tbl, func(t *testing.T) { + identifiers, err := schema.getEntityColumnNames(tbl, tcmap) + assert.NoError(t, err) + names := []string{} + for _, ident := range identifiers { + names = append(names, ident.String()) + } + // compare columns. We disregard order. + expectNames := expectedColNames[tbl][:] + sort.Strings(names) + sort.Strings(expectNames) + assert.Equal(t, expectNames, names) + // emulate the logic that fills known columns for known entities: + tcmap[tbl] = map[string]struct{}{} + for _, name := range names { + tcmap[tbl][name] = struct{}{} + } + }) } } @@ -578,20 +606,47 @@ func TestViewReferences(t *testing.T) { }, }, { - name: "valid *", + name: "valid star", queries: []string{ "create table t1(id int primary key, c char(5))", "create view v1 as select * from t1 where id > 0", }, }, { - name: "valid *, cascaded", + name: "valid star, cascaded", queries: []string{ "create table t1(id int primary key, c char(5))", "create view v1 as select t1.* from t1 where id > 0", "create view v2 as select * from v1 where id > 0", }, }, + { + name: "valid star, two tables, cascaded", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, ts timestamp)", + "create view v1 as select t1.* from t1, t2 where t1.id > 0", + "create view v2 as select * from v1 where c > 0", + }, + }, + { + name: "valid two star, two tables, cascaded", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, ts timestamp)", + "create view v1 as select t1.*, t2.* from t1, t2 where t1.id > 0", + "create view v2 as select * from v1 where c > 0 and ts is not null", + }, + }, + { + name: "valid unqualified star, cascaded", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create table t2(id int primary key, ts timestamp)", + "create view v1 as select * from t1, t2 where t1.id > 0", + "create view v2 as select * from v1 where c > 0 and ts is not null", + }, + }, } for _, ts := range tt { t.Run(ts.name, func(t *testing.T) { From 61de6b0070a3a6b464e504bb529965dde3c1e407 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 25 Jan 2023 12:49:46 +0200 Subject: [PATCH 04/34] removed 'TODO' Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index deb127df811..7f99b936cf7 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -760,8 +760,6 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { return dup, nil } -// TODO - func (s *Schema) ValidateViewReferences() error { var allerrors error availableColumns := tablesColumnsMap{} From 389872c65bb9f4227c066fe4c70e877bdfa03995 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 25 Jan 2023 12:50:35 +0200 Subject: [PATCH 05/34] rename: errs Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 7f99b936cf7..30069b854a5 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -761,7 +761,7 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { } func (s *Schema) ValidateViewReferences() error { - var allerrors error + var errs error availableColumns := tablesColumnsMap{} for _, e := range s.Entities() { @@ -783,18 +783,18 @@ func (s *Schema) ValidateViewReferences() error { tableAliases := map[string]string{} tableReferences := map[string]struct{}{} err := gatherTableInformationForView(view, availableColumns, tableReferences, tableAliases) - allerrors = multierr.Append(allerrors, err) + errs = multierr.Append(errs, err) // Now we can walk the view again and check each column expression // to see if there's an existing column referenced. err = gatherColumnReferenceInformationForView(view, availableColumns, tableReferences, tableAliases) - allerrors = multierr.Append(allerrors, err) + errs = multierr.Append(errs, err) } - return allerrors + return errs } func gatherTableInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { - var allerrors error + var errs error tableErrors := make(map[string]struct{}) err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { switch node := node.(type) { @@ -813,7 +813,7 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns tabl View: view.Name(), Table: aliased, } - allerrors = multierr.Append(allerrors, err) + errs = multierr.Append(errs, err) tableErrors[aliased] = struct{}{} return true, nil } @@ -828,11 +828,11 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns tabl // parsing error. Forget about any view dependency issues we may have found. This is way more important return err } - return allerrors + return errs } func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { - var allerrors error + var errs error qualifiedColumnErrors := make(map[string]map[string]struct{}) unqualifiedColumnErrors := make(map[string]struct{}) @@ -841,10 +841,10 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo case *sqlparser.ColName: if node.Qualifier.IsEmpty() { err := verifyUnqualifiedColumn(view, availableColumns, tableReferences, node.Name, unqualifiedColumnErrors) - allerrors = multierr.Append(allerrors, err) + errs = multierr.Append(errs, err) } else { err := verifyQualifiedColumn(view, availableColumns, tableAliases, node, qualifiedColumnErrors) - allerrors = multierr.Append(allerrors, err) + errs = multierr.Append(errs, err) } } return true, nil @@ -853,7 +853,7 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo // parsing error. Forget about any view dependency issues we may have found. This is way more important return err } - return allerrors + return errs } func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { From 56c8421e6a96d6c593e2426a71a1f1caf418aa05 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 25 Jan 2023 12:53:47 +0200 Subject: [PATCH 06/34] use mutierr Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 30069b854a5..26b370b7767 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -767,7 +767,7 @@ func (s *Schema) ValidateViewReferences() error { for _, e := range s.Entities() { entityColumns, err := s.getEntityColumnNames(e.Name(), availableColumns) if err != nil { - return err + errs = multierr.Append(errs, err) } availableColumns[e.Name()] = map[string]struct{}{} for _, col := range entityColumns { From 11d6379a29dca9d3fa29c5e60999d820699e995f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 25 Jan 2023 12:53:59 +0200 Subject: [PATCH 07/34] excessive test, removed Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/diff_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/vt/schemadiff/diff_test.go b/go/vt/schemadiff/diff_test.go index db45fb789b8..668895a7891 100644 --- a/go/vt/schemadiff/diff_test.go +++ b/go/vt/schemadiff/diff_test.go @@ -667,7 +667,6 @@ func TestDiffSchemas(t *testing.T) { // validate schema1 unaffected by Apply assert.Equal(t, schema1SQL, schema1.ToSQL()) - require.NotNil(t, schema2) appliedDiff, err := schema2.Diff(applied, hints) require.NoError(t, err) assert.Empty(t, appliedDiff) From f4fd6735bddac12543fa55dbdb0974e21ac2fa42 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 25 Jan 2023 12:58:25 +0200 Subject: [PATCH 08/34] go mod tidy Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 12f2bf755c1..46d3a397bb7 100644 --- a/go.mod +++ b/go.mod @@ -113,6 +113,7 @@ require ( github.com/kr/text v0.2.0 github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 github.com/openark/golib v0.0.0-20210531070646-355f37940af8 + go.uber.org/multierr v1.8.0 golang.org/x/exp v0.0.0-20221114191408-850992195362 ) @@ -184,7 +185,6 @@ require ( github.com/tidwall/pretty v1.2.0 // indirect go.opencensus.io v0.24.0 // indirect go.uber.org/atomic v1.10.0 // indirect - go.uber.org/multierr v1.8.0 // indirect go.uber.org/zap v1.23.0 // indirect go4.org/intern v0.0.0-20220617035311-6925f38cc365 // indirect go4.org/unsafe/assume-no-moving-gc v0.0.0-20220617031537-928513b29760 // indirect From a4ae98d27b054eccfd74386099b2f6c997acfdad Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 25 Jan 2023 13:02:11 +0200 Subject: [PATCH 09/34] update to latest mutierr Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 46d3a397bb7..0f88f3cb522 100644 --- a/go.mod +++ b/go.mod @@ -113,7 +113,7 @@ require ( github.com/kr/text v0.2.0 github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 github.com/openark/golib v0.0.0-20210531070646-355f37940af8 - go.uber.org/multierr v1.8.0 + go.uber.org/multierr v1.9.0 golang.org/x/exp v0.0.0-20221114191408-850992195362 ) diff --git a/go.sum b/go.sum index 77ad5500ec6..5c30fd27f9a 100644 --- a/go.sum +++ b/go.sum @@ -816,8 +816,8 @@ go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0 go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= -go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8= -go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak= +go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= +go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo= go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= From 54fb73aadfb070720c2e1aca04ed0d4b49ba9c98 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 26 Jan 2023 11:22:52 +0200 Subject: [PATCH 10/34] avoid setting entity columns in case of error Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 94d8225b4f8..2057c47ee80 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -768,6 +768,7 @@ func (s *Schema) ValidateViewReferences() error { entityColumns, err := s.getEntityColumnNames(e.Name(), availableColumns) if err != nil { errs = multierr.Append(errs, err) + continue } availableColumns[e.Name()] = map[string]struct{}{} for _, col := range entityColumns { From 2d3e70b3489e61ec3836419887ac64933710945f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 30 Jan 2023 10:56:33 +0200 Subject: [PATCH 11/34] grammar Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/errors.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/vt/schemadiff/errors.go b/go/vt/schemadiff/errors.go index 9dfc2ff81be..7ba26017165 100644 --- a/go/vt/schemadiff/errors.go +++ b/go/vt/schemadiff/errors.go @@ -319,13 +319,13 @@ type InvalidColumnReferencedInViewError struct { func (e *InvalidColumnReferencedInViewError) Error() string { switch { case e.Column == "": - return fmt.Sprintf("view %s references non-existing table %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table)) + return fmt.Sprintf("view %s references non-existent table %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table)) case e.Table != "": - return fmt.Sprintf("view %s references non existing column %s.%s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table), sqlescape.EscapeID(e.Column)) + return fmt.Sprintf("view %s references non-existent column %s.%s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table), sqlescape.EscapeID(e.Column)) case e.NonUnique: return fmt.Sprintf("view %s references unqualified but non unique column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) default: - return fmt.Sprintf("view %s references unqualified but non existing column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) + return fmt.Sprintf("view %s references unqualified but non-existent column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) } } From b69ede6aac383e522aa662d50bd04daae9bb9eb2 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 30 Jan 2023 18:04:11 +0200 Subject: [PATCH 12/34] schemadiff: fix scenario where no tables exist in schema and with just views reading from DUAL Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 6 ++++++ go/vt/schemadiff/schema_test.go | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 2057c47ee80..8716558a13e 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -267,6 +267,12 @@ func (s *Schema) normalize() error { // - then we only want views that depend on 1st level views or on tables. These are 2nd level views. // - etc. // we stop when we have been unable to find a view in an iteration. + + // it's possible that the hasn't been any table in this schema. + // for purposes of the algorithm, the iterationLevel must be at least 1 + if iterationLevel < 1 { + iterationLevel = 1 + } for { handledAnyViewsInIteration := false for _, v := range s.views { diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index c24b1afd20b..492eb797b0d 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -127,6 +127,24 @@ func TestNewSchemaFromQueriesUnresolvedAlias(t *testing.T) { assert.EqualError(t, err, (&ViewDependencyUnresolvedError{View: "v7"}).Error()) } +func TestNewSchemaFromQueriesViewFromDual(t *testing.T) { + // v8 does not exist + queries := []string{ + "create view v20 as select 1 from dual2", + } + _, err := NewSchemaFromQueries(queries) + assert.NoError(t, err) +} + +func TestNewSchemaFromQueriesViewFromDualImplicit(t *testing.T) { + // v8 does not exist + queries := []string{ + "create view v20 as select 1", + } + _, err := NewSchemaFromQueries(queries) + assert.NoError(t, err) +} + func TestNewSchemaFromQueriesLoop(t *testing.T) { // v7 and v8 depend on each other queries := append(createQueries, From bc343698f84a769408e28e213838a6a92397ccf0 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 30 Jan 2023 18:11:30 +0200 Subject: [PATCH 13/34] dual, not dual2 Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index 492eb797b0d..e1b48a49407 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -130,7 +130,7 @@ func TestNewSchemaFromQueriesUnresolvedAlias(t *testing.T) { func TestNewSchemaFromQueriesViewFromDual(t *testing.T) { // v8 does not exist queries := []string{ - "create view v20 as select 1 from dual2", + "create view v20 as select 1 from dual", } _, err := NewSchemaFromQueries(queries) assert.NoError(t, err) From bc8799d9359935087ff9583cad78b2f61870d239 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 30 Jan 2023 18:18:55 +0200 Subject: [PATCH 14/34] stripped irrelevant comments Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index e1b48a49407..1d0bbc119c5 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -128,7 +128,6 @@ func TestNewSchemaFromQueriesUnresolvedAlias(t *testing.T) { } func TestNewSchemaFromQueriesViewFromDual(t *testing.T) { - // v8 does not exist queries := []string{ "create view v20 as select 1 from dual", } @@ -137,7 +136,6 @@ func TestNewSchemaFromQueriesViewFromDual(t *testing.T) { } func TestNewSchemaFromQueriesViewFromDualImplicit(t *testing.T) { - // v8 does not exist queries := []string{ "create view v20 as select 1", } From 7f6fd1395607e4221640f2c2c9ec1b59303e7aac Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 6 Feb 2023 08:33:13 +0200 Subject: [PATCH 15/34] using AllErrorRecorder instead of multierr package Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go.mod | 2 +- go.sum | 6 ++++-- go/vt/schemadiff/schema.go | 28 ++++++++++++++-------------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index fc982e8275c..34c3c15d925 100644 --- a/go.mod +++ b/go.mod @@ -112,7 +112,6 @@ require ( github.com/kr/pretty v0.3.1 github.com/kr/text v0.2.0 github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 - go.uber.org/multierr v1.9.0 golang.org/x/exp v0.0.0-20230131160201-f062dba9d201 modernc.org/sqlite v1.20.3 ) @@ -188,6 +187,7 @@ require ( github.com/tidwall/pretty v1.2.0 // indirect go.opencensus.io v0.24.0 // indirect go.uber.org/atomic v1.10.0 // indirect + go.uber.org/multierr v1.8.0 // indirect go.uber.org/zap v1.23.0 // indirect go4.org/intern v0.0.0-20220617035311-6925f38cc365 // indirect go4.org/unsafe/assume-no-moving-gc v0.0.0-20220617031537-928513b29760 // indirect diff --git a/go.sum b/go.sum index e47da287d26..18c48af6ca8 100644 --- a/go.sum +++ b/go.sum @@ -814,12 +814,13 @@ go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= +go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.10.0 h1:9qC72Qh0+3MqyJbAn8YU5xVq1frD8bn3JtD2oXtafVQ= go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= -go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= -go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= +go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8= +go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= @@ -1311,6 +1312,7 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 8fd7dbae72f..3c491c13b81 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -24,9 +24,9 @@ import ( "sort" "strings" - "go.uber.org/multierr" - + "vitess.io/vitess/go/vt/concurrency" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" ) type tablesColumnsMap map[string]map[string]struct{} @@ -772,13 +772,13 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { } func (s *Schema) ValidateViewReferences() error { - var errs error + errs := &concurrency.AllErrorRecorder{} availableColumns := tablesColumnsMap{} for _, e := range s.Entities() { entityColumns, err := s.getEntityColumnNames(e.Name(), availableColumns) if err != nil { - errs = multierr.Append(errs, err) + errs.RecordError(err) continue } availableColumns[e.Name()] = map[string]struct{}{} @@ -795,18 +795,18 @@ func (s *Schema) ValidateViewReferences() error { tableAliases := map[string]string{} tableReferences := map[string]struct{}{} err := gatherTableInformationForView(view, availableColumns, tableReferences, tableAliases) - errs = multierr.Append(errs, err) + errs.RecordError(err) // Now we can walk the view again and check each column expression // to see if there's an existing column referenced. err = gatherColumnReferenceInformationForView(view, availableColumns, tableReferences, tableAliases) - errs = multierr.Append(errs, err) + errs.RecordError(err) } - return errs + return errs.AggrError(vterrors.Aggregate) } func gatherTableInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { - var errs error + errs := &concurrency.AllErrorRecorder{} tableErrors := make(map[string]struct{}) err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { switch node := node.(type) { @@ -825,7 +825,7 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns tabl View: view.Name(), Table: aliased, } - errs = multierr.Append(errs, err) + errs.RecordError(err) tableErrors[aliased] = struct{}{} return true, nil } @@ -840,11 +840,11 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns tabl // parsing error. Forget about any view dependency issues we may have found. This is way more important return err } - return errs + return errs.AggrError(vterrors.Aggregate) } func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { - var errs error + errs := &concurrency.AllErrorRecorder{} qualifiedColumnErrors := make(map[string]map[string]struct{}) unqualifiedColumnErrors := make(map[string]struct{}) @@ -853,10 +853,10 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo case *sqlparser.ColName: if node.Qualifier.IsEmpty() { err := verifyUnqualifiedColumn(view, availableColumns, tableReferences, node.Name, unqualifiedColumnErrors) - errs = multierr.Append(errs, err) + errs.RecordError(err) } else { err := verifyQualifiedColumn(view, availableColumns, tableAliases, node, qualifiedColumnErrors) - errs = multierr.Append(errs, err) + errs.RecordError(err) } } return true, nil @@ -865,7 +865,7 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo // parsing error. Forget about any view dependency issues we may have found. This is way more important return err } - return errs + return errs.AggrError(vterrors.Aggregate) } func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { From 4939d0172d7c2110a4cb1647a954e33a92ea1e7c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Feb 2023 17:09:33 +0200 Subject: [PATCH 16/34] multierr update Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go.mod | 2 +- go.sum | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 34c3c15d925..df53d214477 100644 --- a/go.mod +++ b/go.mod @@ -187,7 +187,7 @@ require ( github.com/tidwall/pretty v1.2.0 // indirect go.opencensus.io v0.24.0 // indirect go.uber.org/atomic v1.10.0 // indirect - go.uber.org/multierr v1.8.0 // indirect + go.uber.org/multierr v1.9.0 // indirect go.uber.org/zap v1.23.0 // indirect go4.org/intern v0.0.0-20220617035311-6925f38cc365 // indirect go4.org/unsafe/assume-no-moving-gc v0.0.0-20220617031537-928513b29760 // indirect diff --git a/go.sum b/go.sum index 18c48af6ca8..e47da287d26 100644 --- a/go.sum +++ b/go.sum @@ -814,13 +814,12 @@ go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= -go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.10.0 h1:9qC72Qh0+3MqyJbAn8YU5xVq1frD8bn3JtD2oXtafVQ= go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= -go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8= -go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak= +go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= +go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.23.0 h1:OjGQ5KQDEUawVHxNwQgPpiypGHOxo2mNZsOqTak4fFY= go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= @@ -1312,7 +1311,6 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 89d6cba5bf8b7a049fee1b1d934c3dad92fc96c8 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 7 Mar 2023 12:05:49 +0200 Subject: [PATCH 17/34] using FakeSI as table-column model Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 90 ++++++++++++++++++++++++--------- go/vt/schemadiff/schema_test.go | 6 +-- 2 files changed, 69 insertions(+), 27 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 3c491c13b81..a812e941218 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -27,9 +27,55 @@ import ( "vitess.io/vitess/go/vt/concurrency" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/semantics" + "vitess.io/vitess/go/vt/vtgate/vindexes" + + querypb "vitess.io/vitess/go/vt/proto/query" ) -type tablesColumnsMap map[string]map[string]struct{} +type declarativeSchemaInformation semantics.FakeSI + +func newDeclarativeSchemaInformation() *declarativeSchemaInformation { + return &declarativeSchemaInformation{ + Tables: make(map[string]*vindexes.Table), + } +} + +func (si *declarativeSchemaInformation) addTable(name string) *vindexes.Table { + tbl := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS(name), + Columns: []vindexes.Column{}, + } + si.Tables[name] = tbl + return tbl +} + +func (si *declarativeSchemaInformation) hasTable(name string) bool { + _, ok := si.Tables[name] + return ok +} + +func (si *declarativeSchemaInformation) addColumn(tableName string, columnName string) *vindexes.Column { + col := &vindexes.Column{ + Name: sqlparser.NewIdentifierCI(columnName), + Type: querypb.Type_VARCHAR, + } + si.Tables[tableName].Columns = append(si.Tables[tableName].Columns, *col) + return col +} + +func (si *declarativeSchemaInformation) hasColumn(tableName string, columnName string) bool { + tbl, ok := si.Tables[tableName] + if !ok { + return false + } + for _, col := range tbl.Columns { + if col.Name.String() == columnName { + return true + } + } + return false +} // Schema represents a database schema, which may contain entities such as tables and views. // Schema is not in itself an Entity, since it is more of a collection of entities. @@ -773,7 +819,7 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { func (s *Schema) ValidateViewReferences() error { errs := &concurrency.AllErrorRecorder{} - availableColumns := tablesColumnsMap{} + availableColumns := newDeclarativeSchemaInformation() for _, e := range s.Entities() { entityColumns, err := s.getEntityColumnNames(e.Name(), availableColumns) @@ -781,14 +827,14 @@ func (s *Schema) ValidateViewReferences() error { errs.RecordError(err) continue } - availableColumns[e.Name()] = map[string]struct{}{} + availableColumns.addTable(e.Name()) for _, col := range entityColumns { - availableColumns[e.Name()][col.Lowered()] = struct{}{} + availableColumns.addColumn(e.Name(), col.Lowered()) } } // Add dual table with no explicit columns for dual style expressions in views. - availableColumns["dual"] = map[string]struct{}{} + availableColumns.addTable("dual") for _, view := range s.Views() { // First gather all referenced tables and table aliases @@ -805,7 +851,7 @@ func (s *Schema) ValidateViewReferences() error { return errs.AggrError(vterrors.Aggregate) } -func gatherTableInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { +func gatherTableInformationForView(view *CreateViewEntity, availableColumns *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { errs := &concurrency.AllErrorRecorder{} tableErrors := make(map[string]struct{}) err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { @@ -816,7 +862,7 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns tabl return true, nil } - if _, ok := availableColumns[aliased]; !ok { + if !availableColumns.hasTable(aliased) { if _, ok := tableErrors[aliased]; ok { // Only show a missing table reference once per view. return true, nil @@ -843,7 +889,7 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns tabl return errs.AggrError(vterrors.Aggregate) } -func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, tableAliases map[string]string) error { +func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { errs := &concurrency.AllErrorRecorder{} qualifiedColumnErrors := make(map[string]map[string]struct{}) unqualifiedColumnErrors := make(map[string]struct{}) @@ -868,20 +914,18 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo return errs.AggrError(vterrors.Aggregate) } -func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns tablesColumnsMap, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { +func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns *declarativeSchemaInformation, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { // In case we have a non-qualified column reference, it needs // to be unique across all referenced tables if it is supposed // to work. columnFound := false for table := range tableReferences { - cols, ok := availableColumns[table] - if !ok { + if !availableColumns.hasTable(table) { // We already dealt with an error for a missing table reference // earlier, so we can ignore it at this point here. return nil } - _, columnInTable := cols[nodeName.Lowered()] - if !columnInTable { + if !availableColumns.hasColumn(table, nodeName.Lowered()) { continue } if columnFound { @@ -917,7 +961,7 @@ func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns tablesColu func verifyQualifiedColumn( view *CreateViewEntity, - availableColumns tablesColumnsMap, + availableColumns *declarativeSchemaInformation, tableAliases map[string]string, node *sqlparser.ColName, columnErrors map[string]map[string]struct{}, ) error { @@ -925,14 +969,12 @@ func verifyQualifiedColumn( if aliased, ok := tableAliases[tableName]; ok { tableName = aliased } - cols, ok := availableColumns[tableName] - if !ok { + if !availableColumns.hasTable(tableName) { // Already dealt with missing tables earlier on, we don't have // any error to add here. return nil } - _, ok = cols[node.Name.Lowered()] - if ok { + if availableColumns.hasColumn(tableName, node.Name.Lowered()) { // Found the column in the table, all good. return nil } @@ -953,7 +995,7 @@ func verifyQualifiedColumn( } // getTableColumnNames returns the names of columns in given table. -func (s *Schema) getEntityColumnNames(entityName string, availableColumns tablesColumnsMap) ( +func (s *Schema) getEntityColumnNames(entityName string, availableColumns *declarativeSchemaInformation) ( columnNames []*sqlparser.IdentifierCI, err error, ) { @@ -984,7 +1026,7 @@ func (s *Schema) getTableColumnNames(t *CreateTableEntity) (columnNames []*sqlpa } // getViewColumnNames returns the names of aliased columns returned by a given view. -func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns tablesColumnsMap) ( +func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns *declarativeSchemaInformation) ( columnNames []*sqlparser.IdentifierCI, err error, ) { @@ -992,8 +1034,8 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns tables switch node := node.(type) { case *sqlparser.StarExpr: if tableName := node.TableName.Name.String(); tableName != "" { - for colName := range availableColumns[tableName] { - name := sqlparser.NewIdentifierCI(colName) + for _, col := range availableColumns.Tables[tableName].Columns { + name := sqlparser.NewIdentifierCI(col.Name.String()) columnNames = append(columnNames, &name) } } else { @@ -1003,8 +1045,8 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns tables } // add all columns from all referenced tables and views for _, entityName := range dependentNames { - for colName := range availableColumns[entityName] { - name := sqlparser.NewIdentifierCI(colName) + for _, col := range availableColumns.Tables[entityName].Columns { + name := sqlparser.NewIdentifierCI(col.Name.String()) columnNames = append(columnNames, &name) } } diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index 25931d90e6f..a8ff7e59074 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -439,7 +439,7 @@ func TestGetEntityColumnNames(t *testing.T) { entities := schema.Entities() require.Equal(t, len(entities), len(expectedColNames)) - tcmap := tablesColumnsMap{} + tcmap := newDeclarativeSchemaInformation() // we test by order of dependency: for _, e := range entities { tbl := e.Name() @@ -456,9 +456,9 @@ func TestGetEntityColumnNames(t *testing.T) { sort.Strings(expectNames) assert.Equal(t, expectNames, names) // emulate the logic that fills known columns for known entities: - tcmap[tbl] = map[string]struct{}{} + tcmap.addTable(tbl) for _, name := range names { - tcmap[tbl][name] = struct{}{} + tcmap.addColumn(tbl, name) } }) } From 2194291d64ad8d5ef7f2fae43a6171b59dde5cbc Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 7 Mar 2023 12:07:32 +0200 Subject: [PATCH 18/34] rename variable Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 46 +++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index a812e941218..63daa61e0c0 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -819,39 +819,39 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) { func (s *Schema) ValidateViewReferences() error { errs := &concurrency.AllErrorRecorder{} - availableColumns := newDeclarativeSchemaInformation() + schemaInformation := newDeclarativeSchemaInformation() for _, e := range s.Entities() { - entityColumns, err := s.getEntityColumnNames(e.Name(), availableColumns) + entityColumns, err := s.getEntityColumnNames(e.Name(), schemaInformation) if err != nil { errs.RecordError(err) continue } - availableColumns.addTable(e.Name()) + schemaInformation.addTable(e.Name()) for _, col := range entityColumns { - availableColumns.addColumn(e.Name(), col.Lowered()) + schemaInformation.addColumn(e.Name(), col.Lowered()) } } // Add dual table with no explicit columns for dual style expressions in views. - availableColumns.addTable("dual") + schemaInformation.addTable("dual") for _, view := range s.Views() { // First gather all referenced tables and table aliases tableAliases := map[string]string{} tableReferences := map[string]struct{}{} - err := gatherTableInformationForView(view, availableColumns, tableReferences, tableAliases) + err := gatherTableInformationForView(view, schemaInformation, tableReferences, tableAliases) errs.RecordError(err) // Now we can walk the view again and check each column expression // to see if there's an existing column referenced. - err = gatherColumnReferenceInformationForView(view, availableColumns, tableReferences, tableAliases) + err = gatherColumnReferenceInformationForView(view, schemaInformation, tableReferences, tableAliases) errs.RecordError(err) } return errs.AggrError(vterrors.Aggregate) } -func gatherTableInformationForView(view *CreateViewEntity, availableColumns *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { +func gatherTableInformationForView(view *CreateViewEntity, schemaInformation *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { errs := &concurrency.AllErrorRecorder{} tableErrors := make(map[string]struct{}) err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { @@ -862,7 +862,7 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns *dec return true, nil } - if !availableColumns.hasTable(aliased) { + if !schemaInformation.hasTable(aliased) { if _, ok := tableErrors[aliased]; ok { // Only show a missing table reference once per view. return true, nil @@ -889,7 +889,7 @@ func gatherTableInformationForView(view *CreateViewEntity, availableColumns *dec return errs.AggrError(vterrors.Aggregate) } -func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableColumns *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { +func gatherColumnReferenceInformationForView(view *CreateViewEntity, schemaInformation *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { errs := &concurrency.AllErrorRecorder{} qualifiedColumnErrors := make(map[string]map[string]struct{}) unqualifiedColumnErrors := make(map[string]struct{}) @@ -898,10 +898,10 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo switch node := node.(type) { case *sqlparser.ColName: if node.Qualifier.IsEmpty() { - err := verifyUnqualifiedColumn(view, availableColumns, tableReferences, node.Name, unqualifiedColumnErrors) + err := verifyUnqualifiedColumn(view, schemaInformation, tableReferences, node.Name, unqualifiedColumnErrors) errs.RecordError(err) } else { - err := verifyQualifiedColumn(view, availableColumns, tableAliases, node, qualifiedColumnErrors) + err := verifyQualifiedColumn(view, schemaInformation, tableAliases, node, qualifiedColumnErrors) errs.RecordError(err) } } @@ -914,18 +914,18 @@ func gatherColumnReferenceInformationForView(view *CreateViewEntity, availableCo return errs.AggrError(vterrors.Aggregate) } -func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns *declarativeSchemaInformation, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { +func verifyUnqualifiedColumn(view *CreateViewEntity, schemaInformation *declarativeSchemaInformation, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { // In case we have a non-qualified column reference, it needs // to be unique across all referenced tables if it is supposed // to work. columnFound := false for table := range tableReferences { - if !availableColumns.hasTable(table) { + if !schemaInformation.hasTable(table) { // We already dealt with an error for a missing table reference // earlier, so we can ignore it at this point here. return nil } - if !availableColumns.hasColumn(table, nodeName.Lowered()) { + if !schemaInformation.hasColumn(table, nodeName.Lowered()) { continue } if columnFound { @@ -961,7 +961,7 @@ func verifyUnqualifiedColumn(view *CreateViewEntity, availableColumns *declarati func verifyQualifiedColumn( view *CreateViewEntity, - availableColumns *declarativeSchemaInformation, + schemaInformation *declarativeSchemaInformation, tableAliases map[string]string, node *sqlparser.ColName, columnErrors map[string]map[string]struct{}, ) error { @@ -969,12 +969,12 @@ func verifyQualifiedColumn( if aliased, ok := tableAliases[tableName]; ok { tableName = aliased } - if !availableColumns.hasTable(tableName) { + if !schemaInformation.hasTable(tableName) { // Already dealt with missing tables earlier on, we don't have // any error to add here. return nil } - if availableColumns.hasColumn(tableName, node.Name.Lowered()) { + if schemaInformation.hasColumn(tableName, node.Name.Lowered()) { // Found the column in the table, all good. return nil } @@ -995,7 +995,7 @@ func verifyQualifiedColumn( } // getTableColumnNames returns the names of columns in given table. -func (s *Schema) getEntityColumnNames(entityName string, availableColumns *declarativeSchemaInformation) ( +func (s *Schema) getEntityColumnNames(entityName string, schemaInformation *declarativeSchemaInformation) ( columnNames []*sqlparser.IdentifierCI, err error, ) { @@ -1012,7 +1012,7 @@ func (s *Schema) getEntityColumnNames(entityName string, availableColumns *decla case *CreateTableEntity: return s.getTableColumnNames(entity), nil case *CreateViewEntity: - return s.getViewColumnNames(entity, availableColumns) + return s.getViewColumnNames(entity, schemaInformation) } return nil, &EntityNotFoundError{Name: entityName} } @@ -1026,7 +1026,7 @@ func (s *Schema) getTableColumnNames(t *CreateTableEntity) (columnNames []*sqlpa } // getViewColumnNames returns the names of aliased columns returned by a given view. -func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns *declarativeSchemaInformation) ( +func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *declarativeSchemaInformation) ( columnNames []*sqlparser.IdentifierCI, err error, ) { @@ -1034,7 +1034,7 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns *decla switch node := node.(type) { case *sqlparser.StarExpr: if tableName := node.TableName.Name.String(); tableName != "" { - for _, col := range availableColumns.Tables[tableName].Columns { + for _, col := range schemaInformation.Tables[tableName].Columns { name := sqlparser.NewIdentifierCI(col.Name.String()) columnNames = append(columnNames, &name) } @@ -1045,7 +1045,7 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, availableColumns *decla } // add all columns from all referenced tables and views for _, entityName := range dependentNames { - for _, col := range availableColumns.Tables[entityName].Columns { + for _, col := range schemaInformation.Tables[entityName].Columns { name := sqlparser.NewIdentifierCI(col.Name.String()) columnNames = append(columnNames, &name) } From 38eab1434a62b56421190f2d6c684a25325799f1 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 7 Mar 2023 12:29:55 +0200 Subject: [PATCH 19/34] clone Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index 63daa61e0c0..a5af5b49e8b 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -1035,8 +1035,8 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl case *sqlparser.StarExpr: if tableName := node.TableName.Name.String(); tableName != "" { for _, col := range schemaInformation.Tables[tableName].Columns { - name := sqlparser.NewIdentifierCI(col.Name.String()) - columnNames = append(columnNames, &name) + name := sqlparser.CloneRefOfIdentifierCI(&col.Name) + columnNames = append(columnNames, name) } } else { dependentNames, err := getViewDependentTableNames(v.CreateView) @@ -1046,8 +1046,8 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl // add all columns from all referenced tables and views for _, entityName := range dependentNames { for _, col := range schemaInformation.Tables[entityName].Columns { - name := sqlparser.NewIdentifierCI(col.Name.String()) - columnNames = append(columnNames, &name) + name := sqlparser.CloneRefOfIdentifierCI(&col.Name) + columnNames = append(columnNames, name) } } } From 7eb83c102550025a7e22e927736c7ef6a0ce4012 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 7 Mar 2023 17:19:52 +0200 Subject: [PATCH 20/34] add InvalidStarExprInViewError Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/errors.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/go/vt/schemadiff/errors.go b/go/vt/schemadiff/errors.go index e63c0e4c3d0..455809ae957 100644 --- a/go/vt/schemadiff/errors.go +++ b/go/vt/schemadiff/errors.go @@ -339,7 +339,7 @@ type InvalidColumnReferencedInViewError struct { View string Table string Column string - NonUnique bool + Ambiguous bool } func (e *InvalidColumnReferencedInViewError) Error() string { @@ -348,13 +348,21 @@ func (e *InvalidColumnReferencedInViewError) Error() string { return fmt.Sprintf("view %s references non-existent table %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table)) case e.Table != "": return fmt.Sprintf("view %s references non-existent column %s.%s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table), sqlescape.EscapeID(e.Column)) - case e.NonUnique: + case e.Ambiguous: return fmt.Sprintf("view %s references unqualified but non unique column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) default: return fmt.Sprintf("view %s references unqualified but non-existent column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) } } +type InvalidStarExprInViewError struct { + View string +} + +func (e *InvalidStarExprInViewError) Error() string { + return fmt.Sprintf("view %s has invalid star expression", sqlescape.EscapeID(e.View)) +} + type EntityNotFoundError struct { Name string } From f98729b4e0aa7e39a839a7825bc9aa4903893547 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 7 Mar 2023 17:40:13 +0200 Subject: [PATCH 21/34] Make 'args()' accessible Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vtgate/semantics/errors.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/go/vt/vtgate/semantics/errors.go b/go/vt/vtgate/semantics/errors.go index 3088fe19430..72c40e22b0d 100644 --- a/go/vt/vtgate/semantics/errors.go +++ b/go/vt/vtgate/semantics/errors.go @@ -201,3 +201,7 @@ func (n *Error) ErrorCode() vtrpcpb.Code { return f.code } } + +func (n *Error) Args() []any { + return n.args +} From 36075ec9b2bcd37d7fc16cc22df7dec9f22e1101 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 7 Mar 2023 17:41:32 +0200 Subject: [PATCH 22/34] using 'go/vt/vtgate/semantics' to analyze view queries. Handling star expression in DUAL Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 247 +++++++--------------------------- go/vt/schemadiff/semantics.go | 62 +++++++++ 2 files changed, 111 insertions(+), 198 deletions(-) create mode 100644 go/vt/schemadiff/semantics.go diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index a5af5b49e8b..be6d6808d82 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -28,55 +28,8 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/semantics" - "vitess.io/vitess/go/vt/vtgate/vindexes" - - querypb "vitess.io/vitess/go/vt/proto/query" ) -type declarativeSchemaInformation semantics.FakeSI - -func newDeclarativeSchemaInformation() *declarativeSchemaInformation { - return &declarativeSchemaInformation{ - Tables: make(map[string]*vindexes.Table), - } -} - -func (si *declarativeSchemaInformation) addTable(name string) *vindexes.Table { - tbl := &vindexes.Table{ - Name: sqlparser.NewIdentifierCS(name), - Columns: []vindexes.Column{}, - } - si.Tables[name] = tbl - return tbl -} - -func (si *declarativeSchemaInformation) hasTable(name string) bool { - _, ok := si.Tables[name] - return ok -} - -func (si *declarativeSchemaInformation) addColumn(tableName string, columnName string) *vindexes.Column { - col := &vindexes.Column{ - Name: sqlparser.NewIdentifierCI(columnName), - Type: querypb.Type_VARCHAR, - } - si.Tables[tableName].Columns = append(si.Tables[tableName].Columns, *col) - return col -} - -func (si *declarativeSchemaInformation) hasColumn(tableName string, columnName string) bool { - tbl, ok := si.Tables[tableName] - if !ok { - return false - } - for _, col := range tbl.Columns { - if col.Name.String() == columnName { - return true - } - } - return false -} - // Schema represents a database schema, which may contain entities such as tables and views. // Schema is not in itself an Entity, since it is more of a collection of entities. type Schema struct { @@ -821,6 +774,9 @@ func (s *Schema) ValidateViewReferences() error { errs := &concurrency.AllErrorRecorder{} schemaInformation := newDeclarativeSchemaInformation() + // Remember that s.Entities() is already ordered by dependency. ie. tables first, then views + // that only depend on those tables (or on dual), then 2nd tier views, etc. + // Thus, the order of iteration below is valid and sufficient, to build for _, e := range s.Entities() { entityColumns, err := s.getEntityColumnNames(e.Name(), schemaInformation) if err != nil { @@ -837,161 +793,51 @@ func (s *Schema) ValidateViewReferences() error { schemaInformation.addTable("dual") for _, view := range s.Views() { - // First gather all referenced tables and table aliases - tableAliases := map[string]string{} - tableReferences := map[string]struct{}{} - err := gatherTableInformationForView(view, schemaInformation, tableReferences, tableAliases) - errs.RecordError(err) - - // Now we can walk the view again and check each column expression - // to see if there's an existing column referenced. - err = gatherColumnReferenceInformationForView(view, schemaInformation, tableReferences, tableAliases) - errs.RecordError(err) - } - return errs.AggrError(vterrors.Aggregate) -} - -func gatherTableInformationForView(view *CreateViewEntity, schemaInformation *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { - errs := &concurrency.AllErrorRecorder{} - tableErrors := make(map[string]struct{}) - err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { - switch node := node.(type) { - case *sqlparser.AliasedTableExpr: - aliased := sqlparser.GetTableName(node.Expr).String() - if aliased == "" { - return true, nil - } - - if !schemaInformation.hasTable(aliased) { - if _, ok := tableErrors[aliased]; ok { - // Only show a missing table reference once per view. - return true, nil - } - err := &InvalidColumnReferencedInViewError{ - View: view.Name(), - Table: aliased, - } - errs.RecordError(err) - tableErrors[aliased] = struct{}{} - return true, nil - } - tableReferences[aliased] = struct{}{} - if node.As.String() != "" { - tableAliases[node.As.String()] = aliased - } - } - return true, nil - }, view.Select) - if err != nil { - // parsing error. Forget about any view dependency issues we may have found. This is way more important - return err - } - return errs.AggrError(vterrors.Aggregate) -} - -func gatherColumnReferenceInformationForView(view *CreateViewEntity, schemaInformation *declarativeSchemaInformation, tableReferences map[string]struct{}, tableAliases map[string]string) error { - errs := &concurrency.AllErrorRecorder{} - qualifiedColumnErrors := make(map[string]map[string]struct{}) - unqualifiedColumnErrors := make(map[string]struct{}) - - err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { - switch node := node.(type) { - case *sqlparser.ColName: - if node.Qualifier.IsEmpty() { - err := verifyUnqualifiedColumn(view, schemaInformation, tableReferences, node.Name, unqualifiedColumnErrors) - errs.RecordError(err) - } else { - err := verifyQualifiedColumn(view, schemaInformation, tableAliases, node, qualifiedColumnErrors) - errs.RecordError(err) + sel := sqlparser.CloneSelectStatement(view.CreateView.Select) // Analyze(), below, rewrites the select; we don't want to actually modify the schema + _, err := semantics.Analyze(sel, semanticKS.Name, schemaInformation) + extractColName := func(arg any) string { + switch arg := arg.(type) { + case string: + return arg + case *sqlparser.ColName: + return arg.Name.String() + default: + return "" // unexpected } } - return true, nil - }, view.Select) - if err != nil { - // parsing error. Forget about any view dependency issues we may have found. This is way more important - return err - } - return errs.AggrError(vterrors.Aggregate) -} - -func verifyUnqualifiedColumn(view *CreateViewEntity, schemaInformation *declarativeSchemaInformation, tableReferences map[string]struct{}, nodeName sqlparser.IdentifierCI, unqualifiedColumnErrors map[string]struct{}) error { - // In case we have a non-qualified column reference, it needs - // to be unique across all referenced tables if it is supposed - // to work. - columnFound := false - for table := range tableReferences { - if !schemaInformation.hasTable(table) { - // We already dealt with an error for a missing table reference - // earlier, so we can ignore it at this point here. - return nil - } - if !schemaInformation.hasColumn(table, nodeName.Lowered()) { - continue - } - if columnFound { - // We already have seen the column before in another table, so - // if we see it again here, that's an error case. - if _, ok := unqualifiedColumnErrors[nodeName.Lowered()]; ok { + formalizeErr := func(err error) error { + if err == nil { return nil } - unqualifiedColumnErrors[nodeName.Lowered()] = struct{}{} - return &InvalidColumnReferencedInViewError{ - View: view.Name(), - Column: nodeName.String(), - NonUnique: true, + semErr, ok := err.(*semantics.Error) + if !ok { + return err + } + if len(semErr.Args()) == 0 { + return err + } + switch semErr.Code { + case semantics.AmbiguousColumn: + if colName := extractColName(semErr.Args()[0]); colName != "" { + return &InvalidColumnReferencedInViewError{ + View: view.Name(), + Column: colName, + Ambiguous: true, + } + } + case semantics.ColumnNotFound: + if colName := extractColName(semErr.Args()[0]); colName != "" { + return &InvalidColumnReferencedInViewError{ + View: view.Name(), + Column: colName, + } + } } + return err } - columnFound = true - } - - // If we've seen the desired column here once, we're all good - if columnFound { - return nil - } - - if _, ok := unqualifiedColumnErrors[nodeName.Lowered()]; ok { - return nil - } - unqualifiedColumnErrors[nodeName.Lowered()] = struct{}{} - return &InvalidColumnReferencedInViewError{ - View: view.Name(), - Column: nodeName.String(), - } -} - -func verifyQualifiedColumn( - view *CreateViewEntity, - schemaInformation *declarativeSchemaInformation, - tableAliases map[string]string, node *sqlparser.ColName, - columnErrors map[string]map[string]struct{}, -) error { - tableName := node.Qualifier.Name.String() - if aliased, ok := tableAliases[tableName]; ok { - tableName = aliased - } - if !schemaInformation.hasTable(tableName) { - // Already dealt with missing tables earlier on, we don't have - // any error to add here. - return nil - } - if schemaInformation.hasColumn(tableName, node.Name.Lowered()) { - // Found the column in the table, all good. - return nil - } - - if _, ok := columnErrors[tableName]; !ok { - columnErrors[tableName] = make(map[string]struct{}) - } - - if _, ok := columnErrors[tableName][node.Name.Lowered()]; ok { - return nil - } - columnErrors[tableName][node.Name.Lowered()] = struct{}{} - return &InvalidColumnReferencedInViewError{ - View: view.Name(), - Table: tableName, - Column: node.Name.String(), + errs.RecordError(formalizeErr(err)) } + return errs.AggrError(vterrors.Aggregate) } // getTableColumnNames returns the names of columns in given table. @@ -1045,12 +891,17 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl } // add all columns from all referenced tables and views for _, entityName := range dependentNames { - for _, col := range schemaInformation.Tables[entityName].Columns { - name := sqlparser.CloneRefOfIdentifierCI(&col.Name) - columnNames = append(columnNames, name) + if schemaInformation.Tables[entityName] != nil { // is nil for dual/DUAL + for _, col := range schemaInformation.Tables[entityName].Columns { + name := sqlparser.CloneRefOfIdentifierCI(&col.Name) + columnNames = append(columnNames, name) + } } } } + if len(columnNames) == 0 { + return false, &InvalidStarExprInViewError{View: v.Name()} + } case *sqlparser.AliasedExpr: if node.As.String() != "" { columnNames = append(columnNames, &node.As) diff --git a/go/vt/schemadiff/semantics.go b/go/vt/schemadiff/semantics.go new file mode 100644 index 00000000000..c4582b51a0c --- /dev/null +++ b/go/vt/schemadiff/semantics.go @@ -0,0 +1,62 @@ +/* +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 schemadiff + +import ( + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/semantics" + "vitess.io/vitess/go/vt/vtgate/vindexes" +) + +// semanticKS is a bogus keyspace, used for consistency purposes. The name is not important +var semanticKS = &vindexes.Keyspace{ + Name: "ks", + Sharded: false, +} + +// declarativeSchemaInformation is a utility wrapper arounf FakeSI, and adds a few utility functions +// to make it more simple and accessible to schemadiff's logic. +type declarativeSchemaInformation struct { + semantics.FakeSI +} + +func newDeclarativeSchemaInformation() *declarativeSchemaInformation { + return &declarativeSchemaInformation{ + semantics.FakeSI{ + Tables: make(map[string]*vindexes.Table), + }, + } +} + +// addTable adds a fake table with an empty column list +func (si *declarativeSchemaInformation) addTable(tableName string) { + tbl := &vindexes.Table{ + Name: sqlparser.NewIdentifierCS(tableName), + Columns: []vindexes.Column{}, + ColumnListAuthoritative: true, + Keyspace: semanticKS, + } + si.Tables[tableName] = tbl +} + +// addColumn adds a fake column with n otype. It assumes the table already exists +func (si *declarativeSchemaInformation) addColumn(tableName string, columnName string) { + col := &vindexes.Column{ + Name: sqlparser.NewIdentifierCI(columnName), + } + si.Tables[tableName].Columns = append(si.Tables[tableName].Columns, *col) +} From 26e957768be54f9d583bda9262c027c30b07e645 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 7 Mar 2023 17:41:49 +0200 Subject: [PATCH 23/34] more test cases Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_test.go | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go index a8ff7e59074..b32aca1a8f7 100644 --- a/go/vt/schemadiff/schema_test.go +++ b/go/vt/schemadiff/schema_test.go @@ -532,7 +532,7 @@ func TestViewReferences(t *testing.T) { "create table t2(id int primary key, c char(5))", "create view v1 as select c from t1, t2 where t2.id=3", }, - expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "c", NonUnique: true}, + expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "c", Ambiguous: true}, }, { name: "invalid unqualified in WHERE clause, multiple tables", @@ -541,7 +541,7 @@ func TestViewReferences(t *testing.T) { "create table t2(id int primary key, c char(5))", "create view v1 as select t2.c from t1, t2 where id=3", }, - expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "id", NonUnique: true}, + expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "id", Ambiguous: true}, }, { name: "valid unqualified, multiple tables", @@ -627,6 +627,29 @@ func TestViewReferences(t *testing.T) { "create view v3 as select num, v1.id, ch from v1 join v2 using (id) where info > 5", }, }, + { + name: "valid dual", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select 1 from dual", + }, + }, + { + name: "invalid dual column", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select id from dual", + }, + expectErr: &InvalidColumnReferencedInViewError{View: "v1", Column: "id"}, + }, + { + name: "invalid dual star", + queries: []string{ + "create table t1(id int primary key, c char(5))", + "create view v1 as select * from dual", + }, + expectErr: &InvalidStarExprInViewError{View: "v1"}, + }, { name: "valid star", queries: []string{ From 5718b58a53ee378581b7bd8428599cd8e64d234a Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:16:13 +0200 Subject: [PATCH 24/34] typo Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/semantics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/schemadiff/semantics.go b/go/vt/schemadiff/semantics.go index c4582b51a0c..e62972780a0 100644 --- a/go/vt/schemadiff/semantics.go +++ b/go/vt/schemadiff/semantics.go @@ -53,7 +53,7 @@ func (si *declarativeSchemaInformation) addTable(tableName string) { si.Tables[tableName] = tbl } -// addColumn adds a fake column with n otype. It assumes the table already exists +// addColumn adds a fake column with no type. It assumes the table already exists func (si *declarativeSchemaInformation) addColumn(tableName string, columnName string) { col := &vindexes.Column{ Name: sqlparser.NewIdentifierCI(columnName), From fe90b317e3b1daf9b4aadac875d8b1304ec7a26c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:20:13 +0200 Subject: [PATCH 25/34] unexpected error Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index be6d6808d82..a46850e5d86 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -25,6 +25,7 @@ import ( "strings" "vitess.io/vitess/go/vt/concurrency" + "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/semantics" @@ -860,7 +861,7 @@ func (s *Schema) getEntityColumnNames(entityName string, schemaInformation *decl case *CreateViewEntity: return s.getViewColumnNames(entity, schemaInformation) } - return nil, &EntityNotFoundError{Name: entityName} + return nil, vterrors.Errorf(vtrpc.Code_INTERNAL, "unexpected entity type for %v", entityName) } // getTableColumnNames returns the names of columns in given table. From db43534c6e1f55fe9c310b4bf27a6b225d989202 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:25:47 +0200 Subject: [PATCH 26/34] use ColumnName() Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index a46850e5d86..f0e94d06d0a 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -904,12 +904,8 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl return false, &InvalidStarExprInViewError{View: v.Name()} } case *sqlparser.AliasedExpr: - if node.As.String() != "" { - columnNames = append(columnNames, &node.As) - } else { - name := sqlparser.NewIdentifierCI(sqlparser.String(node.Expr)) - columnNames = append(columnNames, &name) - } + ci := sqlparser.NewIdentifierCI(node.ColumnName()) + columnNames = append(columnNames, &ci) } return true, nil }, v.Select.GetColumns()) From 3bf836fc754577835b3a07163d49ee082ebfd2e1 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:31:44 +0200 Subject: [PATCH 27/34] iterate columns instead of Walk Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index f0e94d06d0a..bb3bf198381 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -877,7 +877,7 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl columnNames []*sqlparser.IdentifierCI, err error, ) { - err = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + for _, node := range v.Select.GetColumns() { switch node := node.(type) { case *sqlparser.StarExpr: if tableName := node.TableName.Name.String(); tableName != "" { @@ -888,7 +888,7 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl } else { dependentNames, err := getViewDependentTableNames(v.CreateView) if err != nil { - return false, err + return nil, err } // add all columns from all referenced tables and views for _, entityName := range dependentNames { @@ -901,14 +901,14 @@ func (s *Schema) getViewColumnNames(v *CreateViewEntity, schemaInformation *decl } } if len(columnNames) == 0 { - return false, &InvalidStarExprInViewError{View: v.Name()} + return nil, &InvalidStarExprInViewError{View: v.Name()} } case *sqlparser.AliasedExpr: ci := sqlparser.NewIdentifierCI(node.ColumnName()) columnNames = append(columnNames, &ci) } - return true, nil - }, v.Select.GetColumns()) + } + if err != nil { return nil, err } From 1244c0bc54501491a9cd7906abc2f1aefd4c6d26 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Mar 2023 10:13:53 +0200 Subject: [PATCH 28/34] simplify InvalidColumnReferencedInViewError Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/errors.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/go/vt/schemadiff/errors.go b/go/vt/schemadiff/errors.go index 455809ae957..d1c9adeec70 100644 --- a/go/vt/schemadiff/errors.go +++ b/go/vt/schemadiff/errors.go @@ -337,22 +337,15 @@ func (e *ViewDependencyUnresolvedError) Error() string { type InvalidColumnReferencedInViewError struct { View string - Table string Column string Ambiguous bool } func (e *InvalidColumnReferencedInViewError) Error() string { - switch { - case e.Column == "": - return fmt.Sprintf("view %s references non-existent table %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table)) - case e.Table != "": - return fmt.Sprintf("view %s references non-existent column %s.%s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Table), sqlescape.EscapeID(e.Column)) - case e.Ambiguous: + if e.Ambiguous { return fmt.Sprintf("view %s references unqualified but non unique column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) - default: - return fmt.Sprintf("view %s references unqualified but non-existent column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) } + return fmt.Sprintf("view %s references unqualified but non-existent column %s", sqlescape.EscapeID(e.View), sqlescape.EscapeID(e.Column)) } type InvalidStarExprInViewError struct { From 8b07d2174adbfa51acc1303feff5fc4142b7c5b8 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Mar 2023 10:43:32 +0200 Subject: [PATCH 29/34] do not use FakeSI, create my own implementation of semantics.SchemaInformation Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/semantics.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/go/vt/schemadiff/semantics.go b/go/vt/schemadiff/semantics.go index e62972780a0..ef9017d3b25 100644 --- a/go/vt/schemadiff/semantics.go +++ b/go/vt/schemadiff/semantics.go @@ -17,6 +17,9 @@ limitations under the License. package schemadiff import ( + "vitess.io/vitess/go/mysql/collations" + "vitess.io/vitess/go/vt/key" + topodatapb "vitess.io/vitess/go/vt/proto/topodata" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/semantics" "vitess.io/vitess/go/vt/vtgate/vindexes" @@ -28,20 +31,30 @@ var semanticKS = &vindexes.Keyspace{ Sharded: false, } +var _ semantics.SchemaInformation = (*declarativeSchemaInformation)(nil) + // declarativeSchemaInformation is a utility wrapper arounf FakeSI, and adds a few utility functions // to make it more simple and accessible to schemadiff's logic. type declarativeSchemaInformation struct { - semantics.FakeSI + Tables map[string]*vindexes.Table } func newDeclarativeSchemaInformation() *declarativeSchemaInformation { return &declarativeSchemaInformation{ - semantics.FakeSI{ - Tables: make(map[string]*vindexes.Table), - }, + Tables: make(map[string]*vindexes.Table), } } +// FindTableOrVindex implements the SchemaInformation interface +func (si *declarativeSchemaInformation) FindTableOrVindex(tablename sqlparser.TableName) (*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error) { + table := si.Tables[sqlparser.String(tablename)] + return table, nil, "", 0, nil, nil +} + +func (si *declarativeSchemaInformation) ConnCollation() collations.ID { + return 45 +} + // addTable adds a fake table with an empty column list func (si *declarativeSchemaInformation) addTable(tableName string) { tbl := &vindexes.Table{ From 017e10d3f9d6964fef3cf37489506e4deb15191a Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Mar 2023 11:32:06 +0200 Subject: [PATCH 30/34] Refactor: go/vt/vtgate/engine/opcode to reduce semantics package dependencies Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vtgate/engine/concatenate.go | 8 +-- go/vt/vtgate/engine/ordered_aggregate.go | 72 +++---------------- go/vt/vtgate/engine/ordered_aggregate_test.go | 1 + go/vt/vtgate/engine/pullout_subquery.go | 31 +------- go/vt/vtgate/engine/pullout_subquery_test.go | 2 +- go/vt/vtgate/engine/scalar_aggregation.go | 1 + .../vtgate/engine/scalar_aggregation_test.go | 1 + .../vtgate/planbuilder/aggregation_pushing.go | 12 ++-- go/vt/vtgate/planbuilder/expr.go | 13 ++-- go/vt/vtgate/planbuilder/horizon_planning.go | 7 +- .../planbuilder/operators/operator_test.go | 11 ++- .../planbuilder/operators/queryprojection.go | 21 +++--- .../planbuilder/operators/sharded_routing.go | 7 +- .../operators/subquery_planning.go | 5 +- go/vt/vtgate/planbuilder/ordered_aggregate.go | 26 ++++--- go/vt/vtgate/planbuilder/project.go | 3 +- .../vtgate/planbuilder/projection_pushing.go | 3 +- go/vt/vtgate/planbuilder/pullout_subquery.go | 3 +- go/vt/vtgate/planbuilder/rewrite.go | 4 +- go/vt/vtgate/planbuilder/show.go | 18 +++-- go/vt/vtgate/planbuilder/subquery_op.go | 3 +- go/vt/vtgate/semantics/analyzer_test.go | 17 +++-- go/vt/vtgate/semantics/binder.go | 10 +-- go/vt/vtgate/semantics/scoper.go | 3 +- go/vt/vtgate/semantics/typer.go | 6 +- .../tabletmanager/vdiff/table_plan.go | 3 +- .../vdiff/workflow_differ_test.go | 5 +- go/vt/wrangler/vdiff.go | 3 +- go/vt/wrangler/vdiff_test.go | 5 +- 29 files changed, 111 insertions(+), 193 deletions(-) diff --git a/go/vt/vtgate/engine/concatenate.go b/go/vt/vtgate/engine/concatenate.go index 7858ccfc938..4cc5fcc440b 100644 --- a/go/vt/vtgate/engine/concatenate.go +++ b/go/vt/vtgate/engine/concatenate.go @@ -82,8 +82,8 @@ func formatTwoOptionsNicely(a, b string) string { return a + "_" + b } -// ErrWrongNumberOfColumnsInSelect is an error -var ErrWrongNumberOfColumnsInSelect = vterrors.NewErrorf(vtrpcpb.Code_FAILED_PRECONDITION, vterrors.WrongNumberOfColumnsInSelect, "The used SELECT statements have a different number of columns") +// errWrongNumberOfColumnsInSelect is an error +var errWrongNumberOfColumnsInSelect = vterrors.NewErrorf(vtrpcpb.Code_FAILED_PRECONDITION, vterrors.WrongNumberOfColumnsInSelect, "The used SELECT statements have a different number of columns") // TryExecute performs a non-streaming exec. func (c *Concatenate) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) { @@ -106,7 +106,7 @@ func (c *Concatenate) TryExecute(ctx context.Context, vcursor VCursor, bindVars if len(rows) > 0 && len(r.Rows) > 0 && len(rows[0]) != len(r.Rows[0]) { - return nil, ErrWrongNumberOfColumnsInSelect + return nil, errWrongNumberOfColumnsInSelect } rows = append(rows, r.Rows...) @@ -350,7 +350,7 @@ func (c *Concatenate) description() PrimitiveDescription { func (c *Concatenate) compareFields(fields1 []*querypb.Field, fields2 []*querypb.Field) error { if len(fields1) != len(fields2) { - return ErrWrongNumberOfColumnsInSelect + return errWrongNumberOfColumnsInSelect } for i, field1 := range fields1 { if _, found := c.NoNeedToTypeCheck[i]; found { diff --git a/go/vt/vtgate/engine/ordered_aggregate.go b/go/vt/vtgate/engine/ordered_aggregate.go index e5d3057a127..4b7559ca740 100644 --- a/go/vt/vtgate/engine/ordered_aggregate.go +++ b/go/vt/vtgate/engine/ordered_aggregate.go @@ -26,12 +26,20 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/sqltypes" + . "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/evalengine" binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata" querypb "vitess.io/vitess/go/vt/proto/query" ) +var ( + // Some predefined values + countZero = sqltypes.MakeTrusted(sqltypes.Int64, []byte("0")) + countOne = sqltypes.MakeTrusted(sqltypes.Int64, []byte("1")) + sumZero = sqltypes.MakeTrusted(sqltypes.Decimal, []byte("0")) +) + var _ Primitive = (*OrderedAggregate)(nil) // OrderedAggregate is a primitive that expects the underlying primitive @@ -139,70 +147,6 @@ func (ap *AggregateParams) String() string { return fmt.Sprintf("%s%s(%s)", ap.Opcode.String(), dispOrigOp, keyCol) } -// AggregateOpcode is the aggregation Opcode. -type AggregateOpcode int - -// These constants list the possible aggregate opcodes. -const ( - AggregateUnassigned = AggregateOpcode(iota) - AggregateCount - AggregateSum - AggregateMin - AggregateMax - AggregateCountDistinct - AggregateSumDistinct - AggregateGtid - AggregateRandom - AggregateCountStar -) - -var ( - // OpcodeType keeps track of the known output types for different aggregate functions - OpcodeType = map[AggregateOpcode]querypb.Type{ - AggregateCountDistinct: sqltypes.Int64, - AggregateCount: sqltypes.Int64, - AggregateCountStar: sqltypes.Int64, - AggregateSumDistinct: sqltypes.Decimal, - AggregateSum: sqltypes.Decimal, - AggregateGtid: sqltypes.VarChar, - } - // Some predefined values - countZero = sqltypes.MakeTrusted(sqltypes.Int64, []byte("0")) - countOne = sqltypes.MakeTrusted(sqltypes.Int64, []byte("1")) - sumZero = sqltypes.MakeTrusted(sqltypes.Decimal, []byte("0")) -) - -// SupportedAggregates maps the list of supported aggregate -// functions to their opcodes. -var SupportedAggregates = map[string]AggregateOpcode{ - "count": AggregateCount, - "sum": AggregateSum, - "min": AggregateMin, - "max": AggregateMax, - // These functions don't exist in mysql, but are used - // to display the plan. - "count_distinct": AggregateCountDistinct, - "sum_distinct": AggregateSumDistinct, - "vgtid": AggregateGtid, - "count_star": AggregateCountStar, - "random": AggregateRandom, -} - -func (code AggregateOpcode) String() string { - for k, v := range SupportedAggregates { - if v == code { - return k - } - } - return "ERROR" -} - -// MarshalJSON serializes the AggregateOpcode as a JSON string. -// It's used for testing and diagnostics. -func (code AggregateOpcode) MarshalJSON() ([]byte, error) { - return ([]byte)(fmt.Sprintf("\"%s\"", code.String())), nil -} - // RouteType returns a description of the query routing type used by the primitive func (oa *OrderedAggregate) RouteType() string { return oa.Input.RouteType() diff --git a/go/vt/vtgate/engine/ordered_aggregate_test.go b/go/vt/vtgate/engine/ordered_aggregate_test.go index a24ba88fd5e..c522cd03f69 100644 --- a/go/vt/vtgate/engine/ordered_aggregate_test.go +++ b/go/vt/vtgate/engine/ordered_aggregate_test.go @@ -32,6 +32,7 @@ import ( binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata" querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/servenv" + . "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) var collationEnv *collations.Environment diff --git a/go/vt/vtgate/engine/pullout_subquery.go b/go/vt/vtgate/engine/pullout_subquery.go index d3c614b4dd6..545e795ee60 100644 --- a/go/vt/vtgate/engine/pullout_subquery.go +++ b/go/vt/vtgate/engine/pullout_subquery.go @@ -18,10 +18,10 @@ package engine import ( "context" - "fmt" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/vterrors" + . "vitess.io/vitess/go/vt/vtgate/engine/opcode" querypb "vitess.io/vitess/go/vt/proto/query" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" @@ -189,32 +189,3 @@ func (ps *PulloutSubquery) description() PrimitiveDescription { Other: other, } } - -// PulloutOpcode is a number representing the opcode -// for the PulloutSubquery primitive. -type PulloutOpcode int - -// This is the list of PulloutOpcode values. -const ( - PulloutValue = PulloutOpcode(iota) - PulloutIn - PulloutNotIn - PulloutExists -) - -var pulloutName = map[PulloutOpcode]string{ - PulloutValue: "PulloutValue", - PulloutIn: "PulloutIn", - PulloutNotIn: "PulloutNotIn", - PulloutExists: "PulloutExists", -} - -func (code PulloutOpcode) String() string { - return pulloutName[code] -} - -// MarshalJSON serializes the PulloutOpcode as a JSON string. -// It's used for testing and diagnostics. -func (code PulloutOpcode) MarshalJSON() ([]byte, error) { - return ([]byte)(fmt.Sprintf("\"%s\"", code.String())), nil -} diff --git a/go/vt/vtgate/engine/pullout_subquery_test.go b/go/vt/vtgate/engine/pullout_subquery_test.go index d2f57383e99..9b6e7c490f0 100644 --- a/go/vt/vtgate/engine/pullout_subquery_test.go +++ b/go/vt/vtgate/engine/pullout_subquery_test.go @@ -24,8 +24,8 @@ import ( "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" - querypb "vitess.io/vitess/go/vt/proto/query" + . "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) func TestPulloutSubqueryValueGood(t *testing.T) { diff --git a/go/vt/vtgate/engine/scalar_aggregation.go b/go/vt/vtgate/engine/scalar_aggregation.go index a1a76091689..a0cf09bed9d 100644 --- a/go/vt/vtgate/engine/scalar_aggregation.go +++ b/go/vt/vtgate/engine/scalar_aggregation.go @@ -25,6 +25,7 @@ import ( querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/vterrors" + . "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) var _ Primitive = (*ScalarAggregate)(nil) diff --git a/go/vt/vtgate/engine/scalar_aggregation_test.go b/go/vt/vtgate/engine/scalar_aggregation_test.go index 15e72639f3d..ec2fa06c970 100644 --- a/go/vt/vtgate/engine/scalar_aggregation_test.go +++ b/go/vt/vtgate/engine/scalar_aggregation_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" + . "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) func TestEmptyRows(outer *testing.T) { diff --git a/go/vt/vtgate/planbuilder/aggregation_pushing.go b/go/vt/vtgate/planbuilder/aggregation_pushing.go index 15367f9e3e8..cc05c9e8377 100644 --- a/go/vt/vtgate/planbuilder/aggregation_pushing.go +++ b/go/vt/vtgate/planbuilder/aggregation_pushing.go @@ -22,7 +22,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" ) @@ -231,7 +231,7 @@ func countStarAggr() *operators.Aggr { return &operators.Aggr{ Original: &sqlparser.AliasedExpr{Expr: f}, - OpCode: engine.AggregateCountStar, + OpCode: popcode.AggregateCountStar, Alias: "count(*)", } } @@ -420,17 +420,17 @@ func (hp *horizonPlanning) filteredPushAggregation( return newplan, groupingOffsets, outputAggrs, pushed, nil } -func isMinOrMax(in engine.AggregateOpcode) bool { +func isMinOrMax(in popcode.AggregateOpcode) bool { switch in { - case engine.AggregateMin, engine.AggregateMax: + case popcode.AggregateMin, popcode.AggregateMax: return true default: return false } } -func isRandom(in engine.AggregateOpcode) bool { - return in == engine.AggregateRandom +func isRandom(in popcode.AggregateOpcode) bool { + return in == popcode.AggregateRandom } func splitAggregationsToLeftAndRight( diff --git a/go/vt/vtgate/planbuilder/expr.go b/go/vt/vtgate/planbuilder/expr.go index dfbe23b1640..4e1eecc2886 100644 --- a/go/vt/vtgate/planbuilder/expr.go +++ b/go/vt/vtgate/planbuilder/expr.go @@ -20,10 +20,9 @@ import ( "bytes" "fmt" - "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vterrors" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/vindexes" ) @@ -144,7 +143,7 @@ func (pb *primitiveBuilder) findOrigin(expr sqlparser.Expr, reservedVars *sqlpar if !ok { // (subquery) -> :_sq expr = sqlparser.ReplaceExpr(expr, sqi.ast, sqlparser.NewArgument(sqName)) - pullouts = append(pullouts, newPulloutSubquery(engine.PulloutValue, sqName, hasValues, sqi.plan)) + pullouts = append(pullouts, newPulloutSubquery(popcode.PulloutValue, sqName, hasValues, sqi.plan)) continue } switch construct := construct.(type) { @@ -166,7 +165,7 @@ func (pb *primitiveBuilder) findOrigin(expr sqlparser.Expr, reservedVars *sqlpar Right: right, } expr = sqlparser.ReplaceExpr(expr, construct, newExpr) - pullouts = append(pullouts, newPulloutSubquery(engine.PulloutIn, sqName, hasValues, sqi.plan)) + pullouts = append(pullouts, newPulloutSubquery(popcode.PulloutIn, sqName, hasValues, sqi.plan)) } else { // a not in (subquery) -> (:__sq_has_values = 0 or (a not in ::__sq)) left := &sqlparser.ComparisonExpr{ @@ -184,12 +183,12 @@ func (pb *primitiveBuilder) findOrigin(expr sqlparser.Expr, reservedVars *sqlpar Right: right, } expr = sqlparser.ReplaceExpr(expr, construct, newExpr) - pullouts = append(pullouts, newPulloutSubquery(engine.PulloutNotIn, sqName, hasValues, sqi.plan)) + pullouts = append(pullouts, newPulloutSubquery(popcode.PulloutNotIn, sqName, hasValues, sqi.plan)) } case *sqlparser.ExistsExpr: // exists (subquery) -> :__sq_has_values expr = sqlparser.ReplaceExpr(expr, construct, sqlparser.NewArgument(hasValues)) - pullouts = append(pullouts, newPulloutSubquery(engine.PulloutExists, sqName, hasValues, sqi.plan)) + pullouts = append(pullouts, newPulloutSubquery(popcode.PulloutExists, sqName, hasValues, sqi.plan)) } } return pullouts, highestOrigin, expr, nil diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go index 4e33f62ebe5..627c8e0e5c3 100644 --- a/go/vt/vtgate/planbuilder/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/horizon_planning.go @@ -20,6 +20,7 @@ import ( "fmt" "vitess.io/vitess/go/sqltypes" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" @@ -423,11 +424,11 @@ func generateAggregateParams(aggrs []operators.Aggr, aggrParamOffsets [][]offset offset = incomingOffset } - opcode := engine.AggregateSum + opcode := popcode.AggregateSum switch aggr.OpCode { - case engine.AggregateMin, engine.AggregateMax, engine.AggregateRandom: + case popcode.AggregateMin, popcode.AggregateMax, popcode.AggregateRandom: opcode = aggr.OpCode - case engine.AggregateCount, engine.AggregateCountStar, engine.AggregateCountDistinct, engine.AggregateSumDistinct: + case popcode.AggregateCount, popcode.AggregateCountStar, popcode.AggregateCountDistinct, popcode.AggregateSumDistinct: if !pushed { opcode = aggr.OpCode } diff --git a/go/vt/vtgate/planbuilder/operators/operator_test.go b/go/vt/vtgate/planbuilder/operators/operator_test.go index 4ba5588f22e..21475236e94 100644 --- a/go/vt/vtgate/planbuilder/operators/operator_test.go +++ b/go/vt/vtgate/planbuilder/operators/operator_test.go @@ -25,17 +25,14 @@ import ( "strings" "testing" - "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" - - "vitess.io/vitess/go/vt/vtgate/engine" - - "vitess.io/vitess/go/vt/vtgate/vindexes" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/vt/sqlparser" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" + "vitess.io/vitess/go/vt/vtgate/vindexes" ) type lineCountingReader struct { @@ -134,7 +131,7 @@ func testString(op interface{}) string { // TODO case *SubQuery: var inners []string for _, sqOp := range op.Inner { - subquery := fmt.Sprintf("{\n\tType: %s", engine.PulloutOpcode(sqOp.ExtractedSubquery.OpCode).String()) + subquery := fmt.Sprintf("{\n\tType: %s", popcode.PulloutOpcode(sqOp.ExtractedSubquery.OpCode).String()) if sqOp.ExtractedSubquery.GetArgName() != "" { subquery += fmt.Sprintf("\n\tArgName: %s", sqOp.ExtractedSubquery.GetArgName()) } diff --git a/go/vt/vtgate/planbuilder/operators/queryprojection.go b/go/vt/vtgate/planbuilder/operators/queryprojection.go index 8de53a762be..aa462c168d7 100644 --- a/go/vt/vtgate/planbuilder/operators/queryprojection.go +++ b/go/vt/vtgate/planbuilder/operators/queryprojection.go @@ -25,10 +25,9 @@ import ( "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" - "vitess.io/vitess/go/vt/vtgate/engine" - "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) type ( @@ -75,7 +74,7 @@ type ( Aggr struct { Original *sqlparser.AliasedExpr Func sqlparser.AggrFunc - OpCode engine.AggregateOpcode + OpCode popcode.AggregateOpcode Alias string // The index at which the user expects to see this aggregated function. Set to nil, if the user does not ask for it Index *int @@ -551,7 +550,7 @@ orderBy: if !qp.isExprInGroupByExprs(ctx, expr) { out = append(out, Aggr{ Original: aliasedExpr, - OpCode: engine.AggregateRandom, + OpCode: popcode.AggregateRandom, Alias: aliasedExpr.ColumnName(), Index: &idxCopy, }) @@ -563,14 +562,14 @@ orderBy: return nil, vterrors.VT12001("in scatter query: complex aggregate expression") } - opcode, found := engine.SupportedAggregates[strings.ToLower(fnc.AggrName())] + opcode, found := popcode.SupportedAggregates[strings.ToLower(fnc.AggrName())] if !found { return nil, vterrors.VT12001(fmt.Sprintf("in scatter query: aggregation function '%s'", fnc.AggrName())) } - if opcode == engine.AggregateCount { + if opcode == popcode.AggregateCount { if _, isStar := fnc.(*sqlparser.CountStar); isStar { - opcode = engine.AggregateCountStar + opcode = popcode.AggregateCountStar } } @@ -578,10 +577,10 @@ orderBy: if aggr.IsDistinct() { switch opcode { - case engine.AggregateCount: - opcode = engine.AggregateCountDistinct - case engine.AggregateSum: - opcode = engine.AggregateSumDistinct + case popcode.AggregateCount: + opcode = popcode.AggregateCountDistinct + case popcode.AggregateSum: + opcode = popcode.AggregateSumDistinct } } diff --git a/go/vt/vtgate/planbuilder/operators/sharded_routing.go b/go/vt/vtgate/planbuilder/operators/sharded_routing.go index be23698975e..26310637974 100644 --- a/go/vt/vtgate/planbuilder/operators/sharded_routing.go +++ b/go/vt/vtgate/planbuilder/operators/sharded_routing.go @@ -25,6 +25,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" @@ -600,10 +601,10 @@ func makeEvalEngineExpr(ctx *plancontext.PlanningContext, n sqlparser.Expr) eval if extractedSubquery == nil { continue } - switch engine.PulloutOpcode(extractedSubquery.OpCode) { - case engine.PulloutIn, engine.PulloutNotIn: + switch popcode.PulloutOpcode(extractedSubquery.OpCode) { + case popcode.PulloutIn, popcode.PulloutNotIn: expr = sqlparser.NewListArg(extractedSubquery.GetArgName()) - case engine.PulloutValue, engine.PulloutExists: + case popcode.PulloutValue, popcode.PulloutExists: expr = sqlparser.NewArgument(extractedSubquery.GetArgName()) } } diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index 6dd5220e389..af37128eb0c 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -20,6 +20,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/rewrite" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" @@ -61,7 +62,7 @@ func optimizeSubQuery(ctx *plancontext.PlanningContext, op *SubQuery, ts semanti continue } - if inner.ExtractedSubquery.OpCode == int(engine.PulloutExists) { + if inner.ExtractedSubquery.OpCode == int(popcode.PulloutExists) { correlatedTree, err := createCorrelatedSubqueryOp(ctx, innerOp, outer, preds, inner.ExtractedSubquery) if err != nil { return nil, rewrite.SameTree, err @@ -456,7 +457,7 @@ func createCorrelatedSubqueryOp( func canMergeSubqueryOnColumnSelection(ctx *plancontext.PlanningContext, a, b *Route, predicate *sqlparser.ExtractedSubquery) bool { left := predicate.OtherSide opCode := predicate.OpCode - if opCode != int(engine.PulloutValue) && opCode != int(engine.PulloutIn) { + if opCode != int(popcode.PulloutValue) && opCode != int(popcode.PulloutIn) { return false } diff --git a/go/vt/vtgate/planbuilder/ordered_aggregate.go b/go/vt/vtgate/planbuilder/ordered_aggregate.go index 9458e85de66..4d7ca9f1d77 100644 --- a/go/vt/vtgate/planbuilder/ordered_aggregate.go +++ b/go/vt/vtgate/planbuilder/ordered_aggregate.go @@ -21,16 +21,14 @@ import ( "strconv" "strings" - "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" - "vitess.io/vitess/go/mysql/collations" - "vitess.io/vitess/go/sqltypes" - "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" ) var _ logicalPlan = (*orderedAggregate)(nil) @@ -265,7 +263,7 @@ func (oa *orderedAggregate) Primitive() engine.Primitive { func (oa *orderedAggregate) pushAggr(pb *primitiveBuilder, expr *sqlparser.AliasedExpr, origin logicalPlan) (rc *resultColumn, colNumber int, err error) { aggrFunc, _ := expr.Expr.(sqlparser.AggrFunc) - origOpcode := engine.SupportedAggregates[strings.ToLower(aggrFunc.AggrName())] + origOpcode := popcode.SupportedAggregates[strings.ToLower(aggrFunc.AggrName())] opcode := origOpcode if aggrFunc.GetArgs() != nil && len(aggrFunc.GetArgs()) != 1 { @@ -294,10 +292,10 @@ func (oa *orderedAggregate) pushAggr(pb *primitiveBuilder, expr *sqlparser.Alias oa.extraDistinct = col oa.preProcess = true switch opcode { - case engine.AggregateCount: - opcode = engine.AggregateCountDistinct - case engine.AggregateSum: - opcode = engine.AggregateSumDistinct + case popcode.AggregateCount: + opcode = popcode.AggregateCountDistinct + case popcode.AggregateSum: + opcode = popcode.AggregateSumDistinct } oa.aggregates = append(oa.aggregates, &engine.AggregateParams{ Opcode: opcode, @@ -328,7 +326,7 @@ func (oa *orderedAggregate) pushAggr(pb *primitiveBuilder, expr *sqlparser.Alias // needDistinctHandling returns true if oa needs to handle the distinct clause. // If true, it will also return the aliased expression that needs to be pushed // down into the underlying route. -func (oa *orderedAggregate) needDistinctHandling(pb *primitiveBuilder, expr *sqlparser.AliasedExpr, opcode engine.AggregateOpcode) (bool, *sqlparser.AliasedExpr, error) { +func (oa *orderedAggregate) needDistinctHandling(pb *primitiveBuilder, expr *sqlparser.AliasedExpr, opcode popcode.AggregateOpcode) (bool, *sqlparser.AliasedExpr, error) { var innerAliased *sqlparser.AliasedExpr aggr, ok := expr.Expr.(sqlparser.AggrFunc) @@ -339,7 +337,7 @@ func (oa *orderedAggregate) needDistinctHandling(pb *primitiveBuilder, expr *sql if !aggr.IsDistinct() { return false, nil, nil } - if opcode != engine.AggregateCount && opcode != engine.AggregateSum && opcode != engine.AggregateCountStar { + if opcode != popcode.AggregateCount && opcode != popcode.AggregateSum && opcode != popcode.AggregateCountStar { return false, nil, nil } @@ -382,11 +380,11 @@ func (oa *orderedAggregate) Wireup(plan logicalPlan, jt *jointab) error { } for _, key := range oa.aggregates { switch key.Opcode { - case engine.AggregateCount: + case popcode.AggregateCount: if key.Alias == "" { key.Alias = key.Opcode.String() } - key.Opcode = engine.AggregateSum + key.Opcode = popcode.AggregateSum } } diff --git a/go/vt/vtgate/planbuilder/project.go b/go/vt/vtgate/planbuilder/project.go index 6dfea3fcec2..3a8d9e260c8 100644 --- a/go/vt/vtgate/planbuilder/project.go +++ b/go/vt/vtgate/planbuilder/project.go @@ -24,6 +24,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators" ) @@ -74,7 +75,7 @@ func planProjection(pb *primitiveBuilder, in logicalPlan, expr *sqlparser.Aliase // the rows be correctly ordered. case *orderedAggregate: if aggrFunc, isAggregate := expr.Expr.(sqlparser.AggrFunc); isAggregate { - if _, ok := engine.SupportedAggregates[strings.ToLower(aggrFunc.AggrName())]; ok { + if _, ok := popcode.SupportedAggregates[strings.ToLower(aggrFunc.AggrName())]; ok { rc, colNumber, err := node.pushAggr(pb, expr, origin) if err != nil { return nil, nil, 0, err diff --git a/go/vt/vtgate/planbuilder/projection_pushing.go b/go/vt/vtgate/planbuilder/projection_pushing.go index e770ef1c9bd..f0a8d2e2145 100644 --- a/go/vt/vtgate/planbuilder/projection_pushing.go +++ b/go/vt/vtgate/planbuilder/projection_pushing.go @@ -22,6 +22,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" @@ -138,7 +139,7 @@ func pushProjectionIntoOA(ctx *plancontext.PlanningContext, expr *sqlparser.Alia return 0, false, err } node.aggregates = append(node.aggregates, &engine.AggregateParams{ - Opcode: engine.AggregateRandom, + Opcode: popcode.AggregateRandom, Col: offset, Alias: expr.ColumnName(), Expr: expr.Expr, diff --git a/go/vt/vtgate/planbuilder/pullout_subquery.go b/go/vt/vtgate/planbuilder/pullout_subquery.go index a70fb5efdc4..4e1008ff7ae 100644 --- a/go/vt/vtgate/planbuilder/pullout_subquery.go +++ b/go/vt/vtgate/planbuilder/pullout_subquery.go @@ -20,6 +20,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" ) @@ -37,7 +38,7 @@ type pulloutSubquery struct { } // newPulloutSubquery builds a new pulloutSubquery. -func newPulloutSubquery(opcode engine.PulloutOpcode, sqName, hasValues string, subquery logicalPlan) *pulloutSubquery { +func newPulloutSubquery(opcode popcode.PulloutOpcode, sqName, hasValues string, subquery logicalPlan) *pulloutSubquery { return &pulloutSubquery{ subquery: subquery, eSubquery: &engine.PulloutSubquery{ diff --git a/go/vt/vtgate/planbuilder/rewrite.go b/go/vt/vtgate/planbuilder/rewrite.go index 93884ecf863..4a95696c0f0 100644 --- a/go/vt/vtgate/planbuilder/rewrite.go +++ b/go/vt/vtgate/planbuilder/rewrite.go @@ -19,7 +19,7 @@ package planbuilder import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/semantics" ) @@ -131,7 +131,7 @@ func rewriteSubquery(cursor *sqlparser.Cursor, r *rewriter, node *sqlparser.Subq if err != nil { return err } - if semTableSQ.GetArgName() != "" || engine.PulloutOpcode(semTableSQ.OpCode) != engine.PulloutValue { + if semTableSQ.GetArgName() != "" || popcode.PulloutOpcode(semTableSQ.OpCode) != popcode.PulloutValue { return nil } r.inSubquery++ diff --git a/go/vt/vtgate/planbuilder/show.go b/go/vt/vtgate/planbuilder/show.go index a76c4dac333..28e7e707848 100644 --- a/go/vt/vtgate/planbuilder/show.go +++ b/go/vt/vtgate/planbuilder/show.go @@ -22,22 +22,20 @@ import ( "sort" "strings" - "vitess.io/vitess/go/vt/log" - vschemapb "vitess.io/vitess/go/vt/proto/vschema" - "vitess.io/vitess/go/vt/sidecardb" - "vitess.io/vitess/go/mysql/collations" - "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" - - topodatapb "vitess.io/vitess/go/vt/proto/topodata" - "vitess.io/vitess/go/vt/vtgate/vindexes" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" + "vitess.io/vitess/go/vt/log" querypb "vitess.io/vitess/go/vt/proto/query" + topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vschemapb "vitess.io/vitess/go/vt/proto/vschema" + "vitess.io/vitess/go/vt/sidecardb" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" + "vitess.io/vitess/go/vt/vtgate/vindexes" ) const ( @@ -568,7 +566,7 @@ func buildShowVGtidPlan(show *sqlparser.ShowBasic, vschema plancontext.VSchema) PreProcess: true, Aggregates: []*engine.AggregateParams{ { - Opcode: engine.AggregateGtid, + Opcode: popcode.AggregateGtid, Col: 1, Alias: "global vgtid_executed", }, diff --git a/go/vt/vtgate/planbuilder/subquery_op.go b/go/vt/vtgate/planbuilder/subquery_op.go index 93596a5c55e..ed945cbc6ad 100644 --- a/go/vt/vtgate/planbuilder/subquery_op.go +++ b/go/vt/vtgate/planbuilder/subquery_op.go @@ -19,6 +19,7 @@ package planbuilder import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" ) @@ -41,7 +42,7 @@ func transformSubQueryPlan(ctx *plancontext.PlanningContext, op *operators.SubQu if merged != nil { return merged, nil } - plan := newPulloutSubquery(engine.PulloutOpcode(op.Extracted.OpCode), argName, hasValuesArg, innerPlan) + plan := newPulloutSubquery(opcode.PulloutOpcode(op.Extracted.OpCode), argName, hasValuesArg, innerPlan) if err != nil { return nil, err } diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index c88c295ac68..861839abd2f 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -20,13 +20,12 @@ import ( "fmt" "testing" - "vitess.io/vitess/go/vt/vtgate/engine" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/vindexes" ) @@ -528,32 +527,32 @@ func TestScopeForSubqueries(t *testing.T) { func TestSubqueriesMappingWhereClause(t *testing.T) { tcs := []struct { sql string - opCode engine.PulloutOpcode + opCode opcode.PulloutOpcode otherSideName string }{ { sql: "select id from t1 where id in (select uid from t2)", - opCode: engine.PulloutIn, + opCode: opcode.PulloutIn, otherSideName: "id", }, { sql: "select id from t1 where id not in (select uid from t2)", - opCode: engine.PulloutNotIn, + opCode: opcode.PulloutNotIn, otherSideName: "id", }, { sql: "select id from t where col1 = (select uid from t2 order by uid desc limit 1)", - opCode: engine.PulloutValue, + opCode: opcode.PulloutValue, otherSideName: "col1", }, { sql: "select id from t where exists (select uid from t2 where uid = 42)", - opCode: engine.PulloutExists, + opCode: opcode.PulloutExists, otherSideName: "", }, { sql: "select id from t where col1 >= (select uid from t2 where uid = 42)", - opCode: engine.PulloutValue, + opCode: opcode.PulloutValue, otherSideName: "col1", }, } @@ -608,7 +607,7 @@ func TestSubqueriesMappingSelectExprs(t *testing.T) { extractedSubq := semTable.SubqueryRef[subq] assert.True(t, sqlparser.Equals.Expr(extractedSubq.Subquery, subq)) assert.True(t, sqlparser.Equals.Expr(extractedSubq.Original, subq)) - assert.EqualValues(t, engine.PulloutValue, extractedSubq.OpCode) + assert.EqualValues(t, opcode.PulloutValue, extractedSubq.OpCode) }) } } diff --git a/go/vt/vtgate/semantics/binder.go b/go/vt/vtgate/semantics/binder.go index 446489928fc..a3fbf9e6f52 100644 --- a/go/vt/vtgate/semantics/binder.go +++ b/go/vt/vtgate/semantics/binder.go @@ -19,7 +19,7 @@ package semantics import ( "strings" - "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/sqlparser" ) @@ -211,16 +211,16 @@ func (b *binder) createExtractedSubquery(cursor *sqlparser.Cursor, currScope *sc sq := &sqlparser.ExtractedSubquery{ Subquery: subq, Original: subq, - OpCode: int(engine.PulloutValue), + OpCode: int(opcode.PulloutValue), } switch par := cursor.Parent().(type) { case *sqlparser.ComparisonExpr: switch par.Operator { case sqlparser.InOp: - sq.OpCode = int(engine.PulloutIn) + sq.OpCode = int(opcode.PulloutIn) case sqlparser.NotInOp: - sq.OpCode = int(engine.PulloutNotIn) + sq.OpCode = int(opcode.PulloutNotIn) } subq, exp := GetSubqueryAndOtherSide(par) sq.Original = &sqlparser.ComparisonExpr{ @@ -230,7 +230,7 @@ func (b *binder) createExtractedSubquery(cursor *sqlparser.Cursor, currScope *sc } sq.OtherSide = exp case *sqlparser.ExistsExpr: - sq.OpCode = int(engine.PulloutExists) + sq.OpCode = int(opcode.PulloutExists) sq.Original = par } return sq, nil diff --git a/go/vt/vtgate/semantics/scoper.go b/go/vt/vtgate/semantics/scoper.go index adae1319e37..7bc0f28d322 100644 --- a/go/vt/vtgate/semantics/scoper.go +++ b/go/vt/vtgate/semantics/scoper.go @@ -21,7 +21,6 @@ import ( vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/sqlparser" ) @@ -212,7 +211,7 @@ func (s *scoper) createSpecialScopePostProjection(parent sqlparser.SQLNode) erro } thisTableInfo := createVTableInfoForExpressions(sel.SelectExprs, nil /*needed for star expressions*/, s.org) if len(tableInfo.cols) != len(thisTableInfo.cols) { - return engine.ErrWrongNumberOfColumnsInSelect + return vterrors.NewErrorf(vtrpcpb.Code_FAILED_PRECONDITION, vterrors.WrongNumberOfColumnsInSelect, "The used SELECT statements have a different number of columns") } for i, col := range tableInfo.cols { // at this stage, we don't store the actual dependencies, we only store the expressions. diff --git a/go/vt/vtgate/semantics/typer.go b/go/vt/vtgate/semantics/typer.go index 49f6da5bfe0..25b9b58e383 100644 --- a/go/vt/vtgate/semantics/typer.go +++ b/go/vt/vtgate/semantics/typer.go @@ -23,7 +23,7 @@ import ( "vitess.io/vitess/go/sqltypes" querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) // typer is responsible for setting the type for expressions @@ -62,9 +62,9 @@ func (t *typer) up(cursor *sqlparser.Cursor) error { t.exprTypes[node] = floatval } case sqlparser.AggrFunc: - code, ok := engine.SupportedAggregates[strings.ToLower(node.AggrName())] + code, ok := opcode.SupportedAggregates[strings.ToLower(node.AggrName())] if ok { - typ, ok := engine.OpcodeType[code] + typ, ok := opcode.OpcodeType[code] if ok { t.exprTypes[node] = Type{Type: typ} } diff --git a/go/vt/vttablet/tabletmanager/vdiff/table_plan.go b/go/vt/vttablet/tabletmanager/vdiff/table_plan.go index 64993c4eabd..a672de61e0d 100644 --- a/go/vt/vttablet/tabletmanager/vdiff/table_plan.go +++ b/go/vt/vttablet/tabletmanager/vdiff/table_plan.go @@ -26,6 +26,7 @@ import ( querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) type tablePlan struct { @@ -99,7 +100,7 @@ func (td *tableDiffer) buildTablePlan() (*tablePlan, error) { // but will need to be revisited when we add such support to vreplication aggregateFuncType := "sum" aggregates = append(aggregates, &engine.AggregateParams{ - Opcode: engine.SupportedAggregates[aggregateFuncType], + Opcode: opcode.SupportedAggregates[aggregateFuncType], Col: len(sourceSelect.SelectExprs) - 1, }) } diff --git a/go/vt/vttablet/tabletmanager/vdiff/workflow_differ_test.go b/go/vt/vttablet/tabletmanager/vdiff/workflow_differ_test.go index 0f9ad9305ed..27581b6b134 100644 --- a/go/vt/vttablet/tabletmanager/vdiff/workflow_differ_test.go +++ b/go/vt/vttablet/tabletmanager/vdiff/workflow_differ_test.go @@ -33,6 +33,7 @@ import ( tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" ) func TestBuildPlanSuccess(t *testing.T) { @@ -418,10 +419,10 @@ func TestBuildPlanSuccess(t *testing.T) { Direction: sqlparser.AscOrder, }}, aggregates: []*engine.AggregateParams{{ - Opcode: engine.AggregateSum, + Opcode: opcode.AggregateSum, Col: 2, }, { - Opcode: engine.AggregateSum, + Opcode: opcode.AggregateSum, Col: 3, }}, }, diff --git a/go/vt/wrangler/vdiff.go b/go/vt/wrangler/vdiff.go index 5dc8403db41..f627b2e6a69 100644 --- a/go/vt/wrangler/vdiff.go +++ b/go/vt/wrangler/vdiff.go @@ -46,6 +46,7 @@ import ( "vitess.io/vitess/go/vt/vtctl/workflow" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vttablet/tabletconn" "vitess.io/vitess/go/vt/vttablet/tabletmanager/vreplication" @@ -576,7 +577,7 @@ func (df *vdiff) buildTablePlan(table *tabletmanagerdatapb.TableDefinition, quer // but will need to be revisited when we add such support to vreplication aggregateFuncType := "sum" aggregates = append(aggregates, &engine.AggregateParams{ - Opcode: engine.SupportedAggregates[aggregateFuncType], + Opcode: opcode.SupportedAggregates[aggregateFuncType], Col: len(sourceSelect.SelectExprs) - 1, }) } diff --git a/go/vt/wrangler/vdiff_test.go b/go/vt/wrangler/vdiff_test.go index a410f567d62..a3b596f6dd4 100644 --- a/go/vt/wrangler/vdiff_test.go +++ b/go/vt/wrangler/vdiff_test.go @@ -25,6 +25,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/engine/opcode" "context" @@ -395,10 +396,10 @@ func TestVDiffPlanSuccess(t *testing.T) { selectPks: []int{0}, sourcePrimitive: &engine.OrderedAggregate{ Aggregates: []*engine.AggregateParams{{ - Opcode: engine.AggregateSum, + Opcode: opcode.AggregateSum, Col: 2, }, { - Opcode: engine.AggregateSum, + Opcode: opcode.AggregateSum, Col: 3, }}, GroupByKeys: []*engine.GroupByParams{{KeyCol: 0, WeightStringCol: -1}}, From eac28600ab11bd4612f4c1f12c3e191c22e851dd Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Mar 2023 11:34:20 +0200 Subject: [PATCH 31/34] add new package Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vtgate/engine/opcode/constants.go | 97 +++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 go/vt/vtgate/engine/opcode/constants.go diff --git a/go/vt/vtgate/engine/opcode/constants.go b/go/vt/vtgate/engine/opcode/constants.go new file mode 100644 index 00000000000..56d37439a22 --- /dev/null +++ b/go/vt/vtgate/engine/opcode/constants.go @@ -0,0 +1,97 @@ +package opcode + +import ( + "fmt" + + "vitess.io/vitess/go/sqltypes" + querypb "vitess.io/vitess/go/vt/proto/query" +) + +// PulloutOpcode is a number representing the opcode +// for the PulloutSubquery primitive. +type PulloutOpcode int + +// This is the list of PulloutOpcode values. +const ( + PulloutValue = PulloutOpcode(iota) + PulloutIn + PulloutNotIn + PulloutExists +) + +var pulloutName = map[PulloutOpcode]string{ + PulloutValue: "PulloutValue", + PulloutIn: "PulloutIn", + PulloutNotIn: "PulloutNotIn", + PulloutExists: "PulloutExists", +} + +func (code PulloutOpcode) String() string { + return pulloutName[code] +} + +// MarshalJSON serializes the PulloutOpcode as a JSON string. +// It's used for testing and diagnostics. +func (code PulloutOpcode) MarshalJSON() ([]byte, error) { + return ([]byte)(fmt.Sprintf("\"%s\"", code.String())), nil +} + +// AggregateOpcode is the aggregation Opcode. +type AggregateOpcode int + +// These constants list the possible aggregate opcodes. +const ( + AggregateUnassigned = AggregateOpcode(iota) + AggregateCount + AggregateSum + AggregateMin + AggregateMax + AggregateCountDistinct + AggregateSumDistinct + AggregateGtid + AggregateRandom + AggregateCountStar +) + +var ( + // OpcodeType keeps track of the known output types for different aggregate functions + OpcodeType = map[AggregateOpcode]querypb.Type{ + AggregateCountDistinct: sqltypes.Int64, + AggregateCount: sqltypes.Int64, + AggregateCountStar: sqltypes.Int64, + AggregateSumDistinct: sqltypes.Decimal, + AggregateSum: sqltypes.Decimal, + AggregateGtid: sqltypes.VarChar, + } +) + +// SupportedAggregates maps the list of supported aggregate +// functions to their opcodes. +var SupportedAggregates = map[string]AggregateOpcode{ + "count": AggregateCount, + "sum": AggregateSum, + "min": AggregateMin, + "max": AggregateMax, + // These functions don't exist in mysql, but are used + // to display the plan. + "count_distinct": AggregateCountDistinct, + "sum_distinct": AggregateSumDistinct, + "vgtid": AggregateGtid, + "count_star": AggregateCountStar, + "random": AggregateRandom, +} + +func (code AggregateOpcode) String() string { + for k, v := range SupportedAggregates { + if v == code { + return k + } + } + return "ERROR" +} + +// MarshalJSON serializes the AggregateOpcode as a JSON string. +// It's used for testing and diagnostics. +func (code AggregateOpcode) MarshalJSON() ([]byte, error) { + return ([]byte)(fmt.Sprintf("\"%s\"", code.String())), nil +} From 4daa08be9b6a2d4e8e8a739369f048054ffcc509 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Mar 2023 11:36:35 +0200 Subject: [PATCH 32/34] copyright Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/vtgate/engine/opcode/constants.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/go/vt/vtgate/engine/opcode/constants.go b/go/vt/vtgate/engine/opcode/constants.go index 56d37439a22..37b5f9fd288 100644 --- a/go/vt/vtgate/engine/opcode/constants.go +++ b/go/vt/vtgate/engine/opcode/constants.go @@ -1,3 +1,19 @@ +/* +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 opcode import ( From 1699955286031187668aa861cce0de6dea0d5e8a Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Mar 2023 11:38:16 +0200 Subject: [PATCH 33/34] fix function comment Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index bb3bf198381..39ed20024cf 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -841,7 +841,7 @@ func (s *Schema) ValidateViewReferences() error { return errs.AggrError(vterrors.Aggregate) } -// getTableColumnNames returns the names of columns in given table. +// getEntityColumnNames returns the names of columns in given entity (either a table or a view) func (s *Schema) getEntityColumnNames(entityName string, schemaInformation *declarativeSchemaInformation) ( columnNames []*sqlparser.IdentifierCI, err error, From 1c545792cd20ed2bee99a5cff18fab007e96e534 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 20 Mar 2023 15:53:32 +0200 Subject: [PATCH 34/34] empty commit to kick CI Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>