Add support for views in diffing #15
Conversation
Pull Request Test Coverage Report for Build 1753758663
💛 - Coveralls |
There was a problem hiding this comment.
Looks very good to me! I left a lot of 👍 along the way because it was a long change and I wanted to keep track of the parts I reviewed -- you can safely mark those as resolved.
The one thing I'm unsure of: there's excellent test, but I'm not sure I saw a test that has a non-programmatic nature to comparing views in two schemas. To explain better, and I'm not even sure this is generally supported in tengo: is there a way to just have some SQL, as in testdata/integration.go, but have two different versions of this, then have the integration test show a CREATE OR REPLACE VIEW statement? What I see in existence is programmatic comparison of views; but maybe I misunderstand how this works and am missing the point; maybe what I'm asking about already exists.
| @@ -95,6 +95,8 @@ type SchemaDiff struct { | |||
| ToSchema *Schema | |||
| TableDiffs []*TableDiff // a set of statements that, if run, would turn tables in FromSchema into ToSchema | |||
| RoutineDiffs []*RoutineDiff // " but for funcs and procs | |||
|
|
|||
| ViewDiffs []*ViewDiff // a set of statements that, if run, would turn views in FromSchema into ToSchema | |||
| } | ||
|
|
||
| return viewDiffs | ||
| } |
|
|
||
| return vd.Type | ||
| } | ||
|
|
| if stmt, err := vd.Statement(StatementModifiers{}); err != nil || !strings.HasPrefix(stmt, "CREATE OR REPLACE") { | ||
| t.Errorf("Unexpected return value from Statement(): %s / %s", stmt, err) | ||
| } | ||
| } |
| } | ||
|
|
||
| return row.CreateStatement, nil | ||
| } |
| return nil, err | ||
| } | ||
|
|
||
| g, subCtx := errgroup.WithContext(ctx) |
There was a problem hiding this comment.
TIL errgroup and I'm embarrassed.
| } | ||
|
|
||
| func extractAlgorithm(statement string) string { | ||
| r := regexp.MustCompile(`(?i)\bALGORITHM=(?P<Algorithm>MERGE|UNDEFINED|TEMPTABLE)\b`) |
| } | ||
|
|
||
| return views, nil | ||
| } |
| for name, view := range s.ViewsByName() { | ||
| key := ObjectKey{Type: ObjectTypeView, Name: name} | ||
| dict[key] = view.CreateStatement | ||
| } |
| if va, ok := viewAlters[0].(ChangeViewDefinition); !ok { | ||
| t.Errorf("Expected alters[0] to be a ChangeViewDefinition, instead found %T", va) | ||
| } | ||
| } |
|
Thanks for the review @shlomi-noach! Yeah, I can try and do that for sure. Lemme see what I can whip up today. |
|
@iheanyi cool! If this does not play well with how the current tests work, no worries -- we have Vitess tests on top of that, so even though it's one repo apart, we can still have some form of validation. |
|
Perfect, I'll merge it in then if I fall short :) |
|
@shlomi-noach So I feel like given the fact that we can introspect a schema and then test our diffing logic in |
|
@iheanyi sounds good! |
This pull request adds support for view diffing to Tengo. I pretty much followed how we are doing view diffing for things such as tables and such and tried to do the same thing for views where possible. If anything looks off, let me know, but I couldn't think of any better ways of doing this.
TODO: