Skip to content

Commit

Permalink
Support check constraint backend (GoogleCloudPlatform#962)
Browse files Browse the repository at this point in the history 
* Check constraint backend (#9)

Backend Support for Check Constraint

* update api

* fix PR comment

* remove api call to while validating constraints

* Fixed db collation regex to remove collation name from the results

* renamed function name to formatCheckConstraints and added check if constraint name is empty

* fixed PR comments

* added test case for the empty check constraint name

* fix: added regular exprression to match the exact column

* fix: added regular expression to replace table name

* Added test case for the column rename for check constraint

* 1. Refactored GetConstraint function
2. Fixed inforschema unit tests

* added comment at handling case for check constraints

* reverted white spaces

* reverted white spaces

* nit: doesCheckConstraintNameExist

* added comments for doesCheckConstraintNameExist

* PR and UT fixes

* fix UT

* UT fix

* Removed isCheckConstraintsTablePresent function

* moved regex globally

* Fix UT

* fixed UT

* fixed handling of the constraints

* removed unused function

* added unit tests for incompatable name

* Combined unit tests

* added test case for the renaming column having substring of other column

* added the query changes which return distinct value

---------

Co-authored-by: taherkl <taher.lakdawala@ollion.com>
Co-authored-by: Akash Thawait <aakash@ollion.com>
Co-authored-by: Vivek Yadav <vivek.yadav@ollion.com>
  • Loading branch information
@taherkl @akashthawaitcc
4 people committed Dec 23, 2024
1 parent db05c94 commit 9a9e0c4
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 95 deletions.
5 changes: 0 additions & 5 deletions internal/mapping.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions sources/common/toddl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ func (ss *SchemaToSpannerImpl) SchemaToSpannerDDLHelper(conv *internal.Conv, tod
Comment: comment,
Id: srcTable.Id,
}

return nil
}

Expand Down Expand Up @@ -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) {
Expand Down
40 changes: 28 additions & 12 deletions sources/common/toddl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion sources/mysql/infoschema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion sources/mysql/infoschema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
Expand Down
6 changes: 3 additions & 3 deletions webv2/api/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
109 changes: 54 additions & 55 deletions webv2/api/schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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
Expand Down
59 changes: 51 additions & 8 deletions webv2/table/review_table_schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 9a9e0c4

Please sign in to comment.