Skip to content

Commit fe5388f

Browse files
VivekY1098akashthawaitcc
VivekY1098
and
akashthawaitcc
authored
Added invalid expression removal (#55)
* Added invalid expression removal 1. Implemented invalid expression removal * converted error to warning * updated the test for verify api * added the invalid expression removal on pre migration * Update toddl.go --------- Co-authored-by: Vivek Yadav <vivek.yadav@ollion.com> Co-authored-by: Akash Thawait <aakash@ollion.com>
1 parent f6be3eb commit fe5388f

File tree

4 files changed

+49
-10
lines changed

4 files changed

+49
-10
lines changed

‎internal/reports/report_helpers.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func buildTableReportBody(conv *internal.Conv, tableId string, issues map[string
112112
}
113113

114114
// added if to add table level issue
115-
if p.severity == Errors && len(tableLevelIssues) != 0 {
115+
if p.severity == warning && len(tableLevelIssues) != 0 {
116116
for _, issue := range tableLevelIssues {
117117
switch issue {
118118
case internal.TypeMismatch:

‎sources/common/toddl.go‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,23 @@ func RemoveError(tableIssues map[string]internal.TableIssues) map[string]interna
147147

148148
}
149149

150+
// RemoveCheckConstraint this method will remove the constraint which has error
151+
func RemoveCheckConstraint(checkConstraints []ddl.CheckConstraint, expId string) []ddl.CheckConstraint {
152+
var filteredConstraints []ddl.CheckConstraint
153+
154+
for _, checkConstraint := range checkConstraints {
155+
if checkConstraint.ExprId != expId {
156+
filteredConstraints = append(filteredConstraints, checkConstraint)
157+
}
158+
}
159+
return filteredConstraints
160+
}
161+
150162
// GetIssue it will collect all the error and return it
151-
func GetIssue(result internal.VerifyExpressionsOutput) map[string][]internal.SchemaIssue {
163+
func GetIssue(result internal.VerifyExpressionsOutput) (map[string][]internal.SchemaIssue, map[string][]string) {
152164
exprOutputsByTable := make(map[string][]internal.ExpressionVerificationOutput)
153165
issues := make(map[string][]internal.SchemaIssue)
166+
invalidExpIds := make(map[string][]string)
154167
for _, ev := range result.ExpressionVerificationOutputList {
155168
if !ev.Result {
156169
tableId := ev.ExpressionDetail.Metadata["tableId"]
@@ -176,12 +189,13 @@ func GetIssue(result internal.VerifyExpressionsOutput) map[string][]internal.Sch
176189
issue = internal.GenericError
177190
}
178191
issues[tableId] = append(issues[tableId], issue)
192+
invalidExpIds[tableId] = append(invalidExpIds[tableId], ev.ExpressionDetail.ExpressionId)
179193

180194
}
181195

182196
}
183197

184-
return issues
198+
return issues, invalidExpIds
185199

186200
}
187201

@@ -202,7 +216,7 @@ func (ss *SchemaToSpannerImpl) VerifyExpressions(conv *internal.Conv) error {
202216
if result.ExpressionVerificationOutputList == nil {
203217
return result.Err
204218
}
205-
issueTypes := GetIssue(result)
219+
issueTypes, invalidExpIds := GetIssue(result)
206220
if len(issueTypes) > 0 {
207221
for tableId, issues := range issueTypes {
208222

@@ -222,6 +236,16 @@ func (ss *SchemaToSpannerImpl) VerifyExpressions(conv *internal.Conv) error {
222236
}
223237
}
224238
}
239+
240+
if len(invalidExpIds) > 0 {
241+
for tableId, expressionIdList := range invalidExpIds {
242+
for _, expId := range expressionIdList {
243+
spschema := conv.SpSchema[tableId]
244+
spschema.CheckConstraints = RemoveCheckConstraint(spschema.CheckConstraints, expId)
245+
conv.SpSchema[tableId] = spschema
246+
}
247+
}
248+
}
225249
}
226250

227251
return nil

‎sources/common/toddl_test.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,15 @@ func TestVerifyCheckConstraintExpressions(t *testing.T) {
618618
expressions: []ddl.CheckConstraint{
619619
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
620620
{Expr: "(col1 > 18", ExprId: "expr2", Name: "check2"},
621+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
621622
},
622623
expectedResults: []internal.ExpressionVerificationOutput{
623624
{Result: true, Err: nil, ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 0)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr1"}},
624625
{Result: false, Err: errors.New("Syntax error ..."), ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 18", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr2"}},
625626
},
626627
expectedCheckConstraint: []ddl.CheckConstraint{
627628
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
628-
{Expr: "(col1 > 18", ExprId: "expr2", Name: "check2"},
629+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
629630
},
630631
expectedResponse: true,
631632
},
@@ -634,14 +635,15 @@ func TestVerifyCheckConstraintExpressions(t *testing.T) {
634635
expressions: []ddl.CheckConstraint{
635636
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
636637
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
638+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
637639
},
638640
expectedResults: []internal.ExpressionVerificationOutput{
639641
{Result: true, Err: nil, ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 0)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr1"}},
640642
{Result: false, Err: errors.New("Unrecognized name ..."), ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 18)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr2"}},
641643
},
642644
expectedCheckConstraint: []ddl.CheckConstraint{
643645
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
644-
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
646+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
645647
},
646648
expectedResponse: true,
647649
},
@@ -650,14 +652,15 @@ func TestVerifyCheckConstraintExpressions(t *testing.T) {
650652
expressions: []ddl.CheckConstraint{
651653
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
652654
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
655+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
653656
},
654657
expectedResults: []internal.ExpressionVerificationOutput{
655658
{Result: true, Err: nil, ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 0)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr1"}},
656659
{Result: false, Err: errors.New("No matching signature for operator"), ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 18)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr2"}},
657660
},
658661
expectedCheckConstraint: []ddl.CheckConstraint{
659662
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
660-
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
663+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
661664
},
662665
expectedResponse: true,
663666
},
@@ -666,14 +669,15 @@ func TestVerifyCheckConstraintExpressions(t *testing.T) {
666669
expressions: []ddl.CheckConstraint{
667670
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
668671
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
672+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
669673
},
670674
expectedResults: []internal.ExpressionVerificationOutput{
671675
{Result: true, Err: nil, ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 0)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr1"}},
672676
{Result: false, Err: errors.New("Function not found"), ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 18)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr2"}},
673677
},
674678
expectedCheckConstraint: []ddl.CheckConstraint{
675679
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
676-
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
680+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
677681
},
678682
expectedResponse: true,
679683
},
@@ -682,14 +686,15 @@ func TestVerifyCheckConstraintExpressions(t *testing.T) {
682686
expressions: []ddl.CheckConstraint{
683687
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
684688
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
689+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
685690
},
686691
expectedResults: []internal.ExpressionVerificationOutput{
687692
{Result: true, Err: nil, ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 0)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr1"}},
688693
{Result: false, Err: errors.New("Unhandle error"), ExpressionDetail: internal.ExpressionDetail{Expression: "(col1 > 18)", Type: "CHECK", Metadata: map[string]string{"tableId": "t1"}, ExpressionId: "expr2"}},
689694
},
690695
expectedCheckConstraint: []ddl.CheckConstraint{
691696
{Expr: "(col1 > 0)", ExprId: "expr1", Name: "check1"},
692-
{Expr: "(col1 > 18)", ExprId: "expr2", Name: "check2"},
697+
{Expr: "(col1 > 18)", ExprId: "expr3", Name: "check3"},
693698
},
694699
expectedResponse: true,
695700
},

‎webv2/api/schema.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ func (expressionVerificationHandler *ExpressionsVerificationHandler) VerifyCheck
644644
return
645645
}
646646

647-
issueTypes := common.GetIssue(result)
647+
issueTypes, invalidExpIds := common.GetIssue(result)
648648
if len(issueTypes) > 0 {
649649
hasErrorOccurred = true
650650
for tableId, issues := range issueTypes {
@@ -666,6 +666,16 @@ func (expressionVerificationHandler *ExpressionsVerificationHandler) VerifyCheck
666666
}
667667
}
668668

669+
if len(invalidExpIds) > 0 {
670+
for tableId, expressionIdList := range invalidExpIds {
671+
for _, expId := range expressionIdList {
672+
spschema := sessionState.Conv.SpSchema[tableId]
673+
spschema.CheckConstraints = common.RemoveCheckConstraint(spschema.CheckConstraints, expId)
674+
sessionState.Conv.SpSchema[tableId] = spschema
675+
}
676+
}
677+
}
678+
669679
session.UpdateSessionFile()
670680
}
671681
w.WriteHeader(http.StatusOK)

0 commit comments

Comments
 (0)