schemadiff: multi-error in schema normalization

go/errors/errors.go

@@ -0,0 +1,56 @@
+/*
+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 errors
+
+// Wrapped is used to unwrap an error created by errors.Join() in Go 1.20
+type Wrapped interface {
+	Unwrap() []error
+}
+
+// Unwrap unwraps an error created by errors.Join() in Go 1.20, into its components
+func Unwrap(err error) []error {
+	if err == nil {
+		return nil
+	}
+	if u, ok := err.(Wrapped); ok {
+		return u.Unwrap()
+	}
+	return nil
+}
+
+// Unwrap unwraps an error created by errors.Join() in Go 1.20, into its components, recursively
+func UnwrapAll(err error) (errs []error) {
+	if err == nil {
+		return nil
+	}
+	if u, ok := err.(Wrapped); ok {
+		for _, e := range u.Unwrap() {
+			errs = append(errs, UnwrapAll(e)...)
+		}
+		return errs
+	}
+	return []error{err}
+}
+
+// Unwrap unwraps an error created by errors.Join() in Go 1.20, into its components, recursively,
+// and returns one (the first) unwrapped error
+func UnwrapFirst(err error) error {
+	if err == nil {
+		return nil
+	}
+	return UnwrapAll(err)[0]
+}

go/errors/errors_test.go

@@ -0,0 +1,99 @@
+/*
+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 errors
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestUnwrap(t *testing.T) {
+	err1 := errors.New("err1")
+	err2 := errors.New("err2")
+	err3 := errors.New("err3")
+	err4 := errors.New("err4")
+
+	tt := []struct {
+		name              string
+		err               error
+		expectUnwrap      []error
+		expectUnwrapAll   []error
+		expectUnwrapFirst error
+	}{
+		{
+			name:              "nil",
+			expectUnwrap:      nil,
+			expectUnwrapAll:   nil,
+			expectUnwrapFirst: nil,
+		},
+		{
+			name:              "single",
+			err:               err1,
+			expectUnwrap:      nil,
+			expectUnwrapAll:   []error{err1},
+			expectUnwrapFirst: err1,
+		},
+		{
+			name:              "wrapped nil",
+			err:               errors.Join(nil),
+			expectUnwrap:      nil,
+			expectUnwrapAll:   nil,
+			expectUnwrapFirst: nil,
+		},
+		{
+			name:              "single wrapped",
+			err:               errors.Join(err1),
+			expectUnwrap:      []error{err1},
+			expectUnwrapAll:   []error{err1},
+			expectUnwrapFirst: err1,
+		},
+		{
+			name:              "flat wrapped",
+			err:               errors.Join(err1, err2, err3, err4),
+			expectUnwrap:      []error{err1, err2, err3, err4},
+			expectUnwrapAll:   []error{err1, err2, err3, err4},
+			expectUnwrapFirst: err1,
+		},
+		{
+			name:              "double wrapped",
+			err:               errors.Join(errors.Join(err1)),
+			expectUnwrap:      []error{errors.Join(err1)},
+			expectUnwrapAll:   []error{err1},
+			expectUnwrapFirst: err1,
+		},
+		{
+			name:              "double nested wrapped",
+			err:               errors.Join(errors.Join(err1, err2), errors.Join(err3, err4)),
+			expectUnwrap:      []error{errors.Join(err1, err2), errors.Join(err3, err4)},
+			expectUnwrapAll:   []error{err1, err2, err3, err4},
+			expectUnwrapFirst: err1,
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			unwrapped := Unwrap(tc.err)
+			unwrappedAll := UnwrapAll(tc.err)
+			unwrappedFirst := UnwrapFirst(tc.err)
+
+			assert.Equal(t, tc.expectUnwrap, unwrapped)
+			assert.Equal(t, tc.expectUnwrapAll, unwrappedAll)
+			assert.Equal(t, tc.expectUnwrapFirst, unwrappedFirst)
+		})
+	}
+}

go/vt/schemadiff/errors.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 schemadiff
 
 import (

go/vt/schemadiff/schema.go

@@ -24,7 +24,6 @@ import (
 	"sort"
 	"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"
@@ -72,7 +71,7 @@ func NewSchemaFromEntities(entities []Entity) (*Schema, error) {
 		}
 	}
 	if err := schema.normalize(); err != nil {
-		return nil, err
+		return schema, err
 	}
 	return schema, nil
 }
@@ -165,6 +164,8 @@ func getViewDependentTableNames(createView *sqlparser.CreateView) (names []strin
 // normalize is called as part of Schema creation process. The user may only get a hold of normalized schema.
 // It validates some cross-entity constraints, and orders entity based on dependencies (e.g. tables, views that read from tables, 2nd level views, etc.)
 func (s *Schema) normalize() error {
+	var errs error
+
 	s.named = make(map[string]Entity, len(s.tables)+len(s.views))
 	s.sorted = make([]Entity, 0, len(s.tables)+len(s.views))
 	// Verify no two entities share same name
@@ -317,20 +318,20 @@ func (s *Schema) normalize() error {
 			if _, ok := dependencyLevels[v.Name()]; !ok {
 				// We _know_ that in this iteration, at least one view is found unassigned a dependency level.
 				// We return the first one.
-				return &ViewDependencyUnresolvedError{View: v.ViewName.Name.String()}
+				errs = errors.Join(errs, &ViewDependencyUnresolvedError{View: v.ViewName.Name.String()})
 			}
 		}
 	}
 
 	// Validate views' referenced columns: do these columns actually exist in referenced tables/views?
 	if err := s.ValidateViewReferences(); err != nil {
-		return err
+		errs = errors.Join(errs, err)
 	}
 
 	// Validate table definitions
 	for _, t := range s.tables {
 		if err := t.validate(); err != nil {
-			return err
+			return errors.Join(errs, err)
 		}
 	}
 	colTypeEqualForForeignKey := func(a, b *sqlparser.ColumnType) bool {
@@ -374,24 +375,24 @@ func (s *Schema) normalize() error {
 			for i, col := range check.Source {
 				coveredColumn, ok := tableColumns[col.Lowered()]
 				if !ok {
-					return &InvalidColumnInForeignKeyConstraintError{Table: t.Name(), Constraint: cs.Name.String(), Column: col.String()}
+					return errors.Join(errs, &InvalidColumnInForeignKeyConstraintError{Table: t.Name(), Constraint: cs.Name.String(), Column: col.String()})
 				}
 				referencedColumnName := check.ReferenceDefinition.ReferencedColumns[i].Lowered()
 				referencedColumn, ok := referencedColumns[referencedColumnName]
 				if !ok {
-					return &InvalidReferencedColumnInForeignKeyConstraintError{Table: t.Name(), Constraint: cs.Name.String(), ReferencedTable: referencedTableName, ReferencedColumn: referencedColumnName}
+					return errors.Join(errs, &InvalidReferencedColumnInForeignKeyConstraintError{Table: t.Name(), Constraint: cs.Name.String(), ReferencedTable: referencedTableName, ReferencedColumn: referencedColumnName})
 				}
 				if !colTypeEqualForForeignKey(coveredColumn.Type, referencedColumn.Type) {
-					return &ForeignKeyColumnTypeMismatchError{Table: t.Name(), Constraint: cs.Name.String(), Column: coveredColumn.Name.String(), ReferencedTable: referencedTableName, ReferencedColumn: referencedColumnName}
+					return errors.Join(errs, &ForeignKeyColumnTypeMismatchError{Table: t.Name(), Constraint: cs.Name.String(), Column: coveredColumn.Name.String(), ReferencedTable: referencedTableName, ReferencedColumn: referencedColumnName})
 				}
 			}
 
 			if !referencedTable.columnsCoveredByInOrderIndex(check.ReferenceDefinition.ReferencedColumns) {
-				return &MissingForeignKeyReferencedIndexError{Table: t.Name(), Constraint: cs.Name.String(), ReferencedTable: referencedTableName}
+				return errors.Join(errs, &MissingForeignKeyReferencedIndexError{Table: t.Name(), Constraint: cs.Name.String(), ReferencedTable: referencedTableName})
 			}
 		}
 	}
-	return nil
+	return errs
 }
 
 // Entities returns this schema's entities in good order (may be applied without error)
@@ -772,7 +773,7 @@ func (s *Schema) Apply(diffs []EntityDiff) (*Schema, error) {
 }
 
 func (s *Schema) ValidateViewReferences() error {
-	errs := &concurrency.AllErrorRecorder{}
+	var errs error
 	schemaInformation := newDeclarativeSchemaInformation()
 
 	// Remember that s.Entities() is already ordered by dependency. ie. tables first, then views
@@ -781,7 +782,7 @@ func (s *Schema) ValidateViewReferences() error {
 	for _, e := range s.Entities() {
 		entityColumns, err := s.getEntityColumnNames(e.Name(), schemaInformation)
 		if err != nil {
-			errs.RecordError(err)
+			errs = errors.Join(errs, err)
 			continue
 		}
 		schemaInformation.addTable(e.Name())
@@ -836,9 +837,9 @@ func (s *Schema) ValidateViewReferences() error {
 			}
 			return err
 		}
-		errs.RecordError(formalizeErr(err))
+		errs = errors.Join(errs, formalizeErr(err))
 	}
-	return errs.AggrError(vterrors.Aggregate)
+	return errs
 }
 
 // getEntityColumnNames returns the names of columns in given entity (either a table or a view)

go/vt/schemadiff/schema_test.go

@@ -24,6 +24,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"vitess.io/vitess/go/errors"
+
 	"vitess.io/vitess/go/vt/sqlparser"
 )
 
@@ -152,7 +154,8 @@ func TestNewSchemaFromQueriesLoop(t *testing.T) {
 		"create view v8 as select * from t1, v7",
 	)
 	_, err := NewSchemaFromQueries(queries)
-	assert.Error(t, err)
+	require.Error(t, err)
+	err = errors.UnwrapFirst(err)
 	assert.EqualError(t, err, (&ViewDependencyUnresolvedError{View: "v7"}).Error())
 }
 
@@ -700,6 +703,8 @@ func TestViewReferences(t *testing.T) {
 				require.NoError(t, err)
 				require.NotNil(t, schema)
 			} else {
+				require.Error(t, err)
+				err = errors.UnwrapFirst(err)
 				require.Equal(t, ts.expectErr, err, "received error: %v", err)
 			}
 		})