diff --git a/internal/mapping.go b/internal/mapping.go index 93f9f7f37..d98008058 100644 --- a/internal/mapping.go +++ b/internal/mapping.go @@ -251,11 +251,6 @@ func ToSpannerCheckConstraintName(conv *Conv, srcCheckConstraintName string) str return getSpannerValidName(conv, srcCheckConstraintName) } -func GetSpannerValidExpression(cks []ddl.CheckConstraint) []ddl.CheckConstraint { - // TODO validate the check constraints data with batch verification then send back - return cks -} - // conv.UsedNames tracks Spanner names that have been used for table names, foreign key constraints // and indexes. We use this to ensure we generate unique names when // we map from source dbs to Spanner since Spanner requires all these names to be diff --git a/sources/common/toddl.go b/sources/common/toddl.go index 5d8484c8b..a89bd9251 100644 --- a/sources/common/toddl.go +++ b/sources/common/toddl.go @@ -294,7 +294,6 @@ func (ss *SchemaToSpannerImpl) SchemaToSpannerDDLHelper(conv *internal.Conv, tod Comment: comment, Id: srcTable.Id, } - return nil } @@ -354,18 +353,19 @@ func cvtForeignKeys(conv *internal.Conv, spTableName string, srcTableId string, return spKeys } +// cvtCheckConstraint converts check constraints from source to Spanner. func cvtCheckConstraint(conv *internal.Conv, srcKeys []schema.CheckConstraint) []ddl.CheckConstraint { - var spcks []ddl.CheckConstraint - - for _, cks := range srcKeys { - spcks = append(spcks, ddl.CheckConstraint{ - Id: cks.Id, - Name: internal.ToSpannerCheckConstraintName(conv, cks.Name), - Expr: cks.Expr, - ExprId: cks.ExprId, + var spcc []ddl.CheckConstraint + + for _, cc := range srcKeys { + spcc = append(spcc, ddl.CheckConstraint{ + Id: cc.Id, + Name: internal.ToSpannerCheckConstraintName(conv, cc.Name), + Expr: cc.Expr, + ExprId: cc.ExprId, }) } - return spcks + return spcc } func CvtForeignKeysHelper(conv *internal.Conv, spTableName string, srcTableId string, srcKey schema.ForeignKey, isRestore bool) (ddl.Foreignkey, error) { diff --git a/sources/common/toddl_test.go b/sources/common/toddl_test.go index 734bb0bf1..4f2fff4a0 100644 --- a/sources/common/toddl_test.go +++ b/sources/common/toddl_test.go @@ -438,26 +438,42 @@ func Test_cvtCheckContraint(t *testing.T) { conv := internal.MakeConv() srcSchema := []schema.CheckConstraint{ { - Id: "ck1", - Name: "check_1", - Expr: "age > 0", + Id: "cc1", + Name: "check_1", + Expr: "age > 0", + ExprId: "expr1", }, { - Id: "ck1", - Name: "check_2", - Expr: "age < 99", + Id: "cc2", + Name: "check_2", + Expr: "age < 99", + ExprId: "expr2", + }, + { + Id: "cc3", + Name: "@invalid_name", // incompatabile name + Expr: "age != 0", + ExprId: "expr3", }, } spSchema := []ddl.CheckConstraint{ { - Id: "ck1", - Name: "check_1", - Expr: "age > 0", + Id: "cc1", + Name: "check_1", + Expr: "age > 0", + ExprId: "expr1", + }, + { + Id: "cc2", + Name: "check_2", + Expr: "age < 99", + ExprId: "expr2", }, { - Id: "ck1", - Name: "check_2", - Expr: "age < 99", + Id: "cc3", + Name: "Ainvalid_name", + Expr: "age != 0", + ExprId: "expr3", }, } result := cvtCheckConstraint(conv, srcSchema) diff --git a/sources/mysql/infoschema.go b/sources/mysql/infoschema.go index c41582cde..cad3c49a2 100644 --- a/sources/mysql/infoschema.go +++ b/sources/mysql/infoschema.go @@ -324,7 +324,6 @@ func (isi InfoSchemaImpl) processRow( // Case added to handle check constraints case "CHECK": checkClause = collationRegex.ReplaceAllString(checkClause, "") - // constraintName := fmt.Sprintf("%s_check", col) *checkKeys = append(*checkKeys, schema.CheckConstraint{Name: constraintName, Expr: checkClause, ExprId: internal.GenerateCheckConstrainstExprId(), Id: internal.GenerateCheckConstrainstId()}) default: m[col] = append(m[col], constraintType) diff --git a/sources/mysql/infoschema_test.go b/sources/mysql/infoschema_test.go index 4a4e95634..ca5a47ae1 100644 --- a/sources/mysql/infoschema_test.go +++ b/sources/mysql/infoschema_test.go @@ -501,7 +501,7 @@ func TestProcessData_MultiCol(t *testing.T) { } internal.AssertSpSchema(conv, t, expectedSchema, stripSchemaComments(conv.SpSchema)) columnLevelIssues := map[string][]internal.SchemaIssue{ - "c5": { + "c49": { 2, }, } diff --git a/webv2/api/schema.go b/webv2/api/schema.go index 7639f10df..e89146897 100644 --- a/webv2/api/schema.go +++ b/webv2/api/schema.go @@ -512,14 +512,14 @@ func UpdateCheckConstraint(w http.ResponseWriter, r *http.Request) { sessionState.Conv.ConvLock.Lock() defer sessionState.Conv.ConvLock.Unlock() - newCKs := []ddl.CheckConstraint{} - if err = json.Unmarshal(reqBody, &newCKs); err != nil { + newCc := []ddl.CheckConstraint{} + if err = json.Unmarshal(reqBody, &newCc); err != nil { http.Error(w, fmt.Sprintf("Request Body parse error : %v", err), http.StatusBadRequest) return } sp := sessionState.Conv.SpSchema[tableId] - sp.CheckConstraints = newCKs + sp.CheckConstraints = newCc sessionState.Conv.SpSchema[tableId] = sp session.UpdateSessionFile() diff --git a/webv2/api/schema_test.go b/webv2/api/schema_test.go index 97bfa5330..198cc0210 100644 --- a/webv2/api/schema_test.go +++ b/webv2/api/schema_test.go @@ -2548,64 +2548,79 @@ func TestGetAutoGenMapMySQL(t *testing.T) { } } + func TestUpdateCheckConstraint(t *testing.T) { - sessionState := session.GetSessionState() - sessionState.Driver = constants.MYSQL - sessionState.Conv = internal.MakeConv() + t.Run("ValidCheckConstraints", func(t *testing.T) { + sessionState := session.GetSessionState() + sessionState.Driver = constants.MYSQL + sessionState.Conv = internal.MakeConv() - tableID := "table1" + tableID := "table1" - expectedCheckConstraint := []ddl.CheckConstraint{ - {Id: "ck1", Name: "check_1", Expr: "(age > 18)"}, - {Id: "ck2", Name: "check_2", Expr: "(age < 99)"}, - } + expectedCheckConstraint := []ddl.CheckConstraint{ + {Id: "cc1", Name: "check_1", Expr: "(age > 18)"}, + {Id: "cc2", Name: "check_2", Expr: "(age < 99)"}, + } - checkConstraints := []schema.CheckConstraint{ - {Id: "ck1", Name: "check_1", Expr: "(age > 18)"}, - {Id: "ck2", Name: "check_2", Expr: "(age < 99)"}, - } + checkConstraints := []schema.CheckConstraint{ + {Id: "cc1", Name: "check_1", Expr: "(age > 18)"}, + {Id: "cc2", Name: "check_2", Expr: "(age < 99)"}, + } - body, err := json.Marshal(checkConstraints) - assert.NoError(t, err) + body, err := json.Marshal(checkConstraints) + assert.NoError(t, err) - req, err := http.NewRequest("POST", "update/cc", bytes.NewBuffer(body)) - assert.NoError(t, err) + req, err := http.NewRequest("POST", "update/cc", bytes.NewBuffer(body)) + assert.NoError(t, err) - q := req.URL.Query() - q.Add("table", tableID) - req.URL.RawQuery = q.Encode() + q := req.URL.Query() + q.Add("table", tableID) + req.URL.RawQuery = q.Encode() - rr := httptest.NewRecorder() + rr := httptest.NewRecorder() + handler := http.HandlerFunc(api.UpdateCheckConstraint) + handler.ServeHTTP(rr, req) - handler := http.HandlerFunc(api.UpdateCheckConstraint) + assert.Equal(t, http.StatusOK, rr.Code) - handler.ServeHTTP(rr, req) + updatedSp := sessionState.Conv.SpSchema[tableID] + assert.Equal(t, expectedCheckConstraint, updatedSp.CheckConstraints) + }) - assert.Equal(t, http.StatusOK, rr.Code) + t.Run("ParseError", func(t *testing.T) { + sessionState := session.GetSessionState() + sessionState.Driver = constants.MYSQL + sessionState.Conv = internal.MakeConv() - updatedSp := sessionState.Conv.SpSchema[tableID] + invalidJSON := "invalid json body" - assert.Equal(t, expectedCheckConstraint, updatedSp.CheckConstraints) -} + rr := httptest.NewRecorder() + req, err := http.NewRequest("POST", "update/cc", io.NopCloser(strings.NewReader(invalidJSON))) + assert.NoError(t, err) -func TestUpdateCheckConstraint_ParseError(t *testing.T) { - sessionState := session.GetSessionState() - sessionState.Driver = constants.MYSQL - sessionState.Conv = internal.MakeConv() + handler := http.HandlerFunc(api.UpdateCheckConstraint) + handler.ServeHTTP(rr, req) - invalidJSON := "invalid json body" + assert.Equal(t, http.StatusBadRequest, rr.Code) - rr := httptest.NewRecorder() - req, err := http.NewRequest("POST", "update/cc", io.NopCloser(strings.NewReader(invalidJSON))) - assert.NoError(t, err) + expectedErrorMessage := "Request Body parse error" + assert.Contains(t, rr.Body.String(), expectedErrorMessage) + }) - handler := http.HandlerFunc(api.UpdateCheckConstraint) - handler.ServeHTTP(rr, req) + t.Run("ImproperSession", func(t *testing.T) { + sessionState := session.GetSessionState() + sessionState.Conv = nil // Simulate no conversion + + rr := httptest.NewRecorder() + req, err := http.NewRequest("POST", "update/cc", io.NopCloser(errReader{})) + assert.NoError(t, err) - assert.Equal(t, http.StatusBadRequest, rr.Code) + handler := http.HandlerFunc(api.UpdateCheckConstraint) + handler.ServeHTTP(rr, req) - expectedErrorMessage := "Request Body parse error" - assert.Contains(t, rr.Body.String(), expectedErrorMessage) + assert.Equal(t, http.StatusInternalServerError, rr.Code) + assert.Contains(t, rr.Body.String(), "Schema is not converted or Driver is not configured properly") + }) } type errReader struct{} @@ -2614,22 +2629,6 @@ func (errReader) Read(p []byte) (n int, err error) { return 0, fmt.Errorf("simulated read error") } -func TestUpdateCheckConstraint_ImproperSession(t *testing.T) { - sessionState := session.GetSessionState() - sessionState.Conv = nil // Simulate no conversion - - rr := httptest.NewRecorder() - req, err := http.NewRequest("POST", "update/cc", io.NopCloser(errReader{})) - assert.NoError(t, err) - - handler := http.HandlerFunc(api.UpdateCheckConstraint) - handler.ServeHTTP(rr, req) - - assert.Equal(t, http.StatusInternalServerError, rr.Code) - assert.Contains(t, rr.Body.String(), "Schema is not converted or Driver is not configured properly") - -} - func TestVerifyCheckConstraintExpressions(t *testing.T) { tests := []struct { name string diff --git a/webv2/table/review_table_schema_test.go b/webv2/table/review_table_schema_test.go index 0373cb39d..34bfcf339 100644 --- a/webv2/table/review_table_schema_test.go +++ b/webv2/table/review_table_schema_test.go @@ -2096,14 +2096,9 @@ func TestReviewTableSchema(t *testing.T) { }, }, { - name: "rename constraints column", - tableId: "t1", - payload: ` - { - "UpdateCols":{ - "c1": { "Rename": "aa" } - } - }`, + name: "rename constraints column", + tableId: "t1", + payload: `{"UpdateCols":{"c1": { "Rename": "aa" }}}`, statusCode: http.StatusOK, conv: &internal.Conv{ SpSchema: map[string]ddl.CreateTable{ @@ -2148,6 +2143,54 @@ func TestReviewTableSchema(t *testing.T) { }, }, }, + { + name: "exact match of column name", + tableId: "t1", + payload: `{"UpdateCols":{"c2": { "Rename": "c2" }}}`, + statusCode: http.StatusOK, + conv: &internal.Conv{ + SpSchema: map[string]ddl.CreateTable{ + "t1": { + Name: "t1", + ColIds: []string{"c1", "c2", "c3"}, + ColDefs: map[string]ddl.ColumnDef{ + "c1": {Name: "c1", Id: "c1", T: ddl.Type{Name: ddl.String, Len: ddl.MaxLength}}, + "c2": {Name: "c1_1", Id: "c2", T: ddl.Type{Name: ddl.String, Len: ddl.MaxLength}}, + "c3": {Name: "c3", Id: "c3", T: ddl.Type{Name: ddl.Int64}}, + }, + PrimaryKeys: []ddl.IndexKey{{ColId: "c1"}}, + CheckConstraints: []ddl.CheckConstraint{{ + Name: "check1", + Expr: "c1_1 > 0", + }}, + }, + }, + Audit: internal.Audit{ + MigrationType: migration.MigrationData_MIGRATION_TYPE_UNSPECIFIED.Enum(), + }, + }, + expectedConv: &internal.Conv{ + SpSchema: map[string]ddl.CreateTable{ + "t1": { + Name: "t1", + ColIds: []string{"c1", "c2", "c3"}, + ColDefs: map[string]ddl.ColumnDef{ + "c1": {Name: "c1", Id: "c1", T: ddl.Type{Name: ddl.String, Len: ddl.MaxLength}}, + "c2": {Name: "c2", Id: "c2", T: ddl.Type{Name: ddl.String, Len: ddl.MaxLength}}, + "c3": {Name: "c3", Id: "c3", T: ddl.Type{Name: ddl.Int64}}, + }, + PrimaryKeys: []ddl.IndexKey{{ColId: "c1"}}, + CheckConstraints: []ddl.CheckConstraint{{ + Name: "check1", + Expr: "c2 > 0", + }}, + }, + }, + Audit: internal.Audit{ + MigrationType: migration.MigrationData_MIGRATION_TYPE_UNSPECIFIED.Enum(), + }, + }, + }, } for _, tc := range tc {