-
Notifications
You must be signed in to change notification settings - Fork 203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposed schemadiff
blog post
#1435
Conversation
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
✅ Deploy Preview for vitess ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
However, there are disadvantages, as well: | ||
|
||
- Complex expressions are left, well, complex. A `CREATE VIEW ...` statement is really just a long SQL text. Which tables or views are referenced in our `VIEW` definition? Which columns? | ||
- Not everything is normalized and formalized. We know `int(10)` and `int(11)` are really the same. But as MySQL goes, the column type is different. Contract this with `varchar(10)` and `varchar(11)`, where the type is truly different. It still takes some higher level logic to understand what is a real diff and what isn't. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe good to call out here that this is deprecated for MySQL 8.0 and link to that it doesn't mean anything anymore? Feels like it's maybe something people don't really all know yet since I see ints with lengths still in so many places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added link to https://dev.mysql.com/worklog/task/?id=13127 ; it's the best I could find that talks not only about ZEROFILL
but also about precision.
|
||
Vitess is a sharding and infrastructure framework running on top of MySQL, and masquerades as a MySQL server to route queries into relevant shards. It thus obviously must be able to parse MySQL's SQL dialect. [`sqlparser`](https://github.com/vitessio/vitess/tree/main/go/vt/sqlparser) is the Vitess library that does so. | ||
|
||
`sqlparser` utilizes a [classic yacc file](https://github.com/vitessio/vitess/blob/main/go/vt/sqlparser/sql.y) to parse SQL into an Abstract Syntax Tree (AST), with `golang` structs generated and populated by the parser. For example, a SQL `CREATE TABLE` statement is parsed into a [`CreateTable`](https://github.com/vitessio/vitess/blob/157857a4c5ee6dcb44e7de08894db8334750746e/go/vt/sqlparser/ast.go#L509-L517) instance: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe useful to also link to https://en.wikipedia.org/wiki/Yacc here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There's a few things to normalize here. Inputs can come in different shapes and sizes, and still mean the same thing. Even MySQL itself presents different output for textual columns based on which version the table was created in. In the above table definition, we can normalize the following: | ||
|
||
- The canonical way to declare a `primary key` is as an index definition, not as part of the column definition. | ||
- `int(12)` is just an `int`. `12` does not matter. Interestingly, some ORMs do have special treatment for `int(1)`, as an indication to a boolean value. `schemadiff` accommodates that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example has int(11)
but you talk about int(12)
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
- The canonical way to declare a `primary key` is as an index definition, not as part of the column definition. | ||
- `int(12)` is just an `int`. `12` does not matter. Interestingly, some ORMs do have special treatment for `int(1)`, as an indication to a boolean value. `schemadiff` accommodates that. | ||
- It looks like `i` is `NULL`able (because it does not say `not null`). Which means its default value is `null` even if we don't explicitly say that. | ||
- `v`'s collation, and thereby character set, agree with the table's collation. It can be removed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we indicate that this is a choice we made for schemadiff
to have it be simpler for the user to read. Vs. MySQL, which is often very verbose here in show create table
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, in next paragraph.
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
I will make minor edits to the code samples per vitessio/vitess#12551 and vitessio/vitess#12885 |
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! Very well-structured and informative.
Lots of wording / re-phrasing / grammar suggestions though.
@shlomi-noach can you rename the file and change the date so that we can plan to publish this on April 24, Monday? I assume all reviews can be addressed by then. |
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Co-authored-by: Deepthi Sigireddi <deepthi@planetscale.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@deepthi thank you, I believe I've addressed all of your comments. File is renamed and edited to be published on April 24th. The new preview link is now: https://deploy-preview-1435--vitess.netlify.app/blog/2023-04-24-schemadiff/ |
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Updated code samples to reflect changes merged in vitessio/vitess#12885 |
The old file still needs to be deleted, there are now 2 files in this PR. |
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
That's bizzarre. I used |
Does that mean the PR can be merged any time before Monday April 24, or does it need to be merged on the 24th? |
If you merge it before Apr 24, it won't appear on the website right away. The first website deploy on or after Apr 24 will publish it. |
Published URL: https://vitess.io/blog/2023-04-24-schemadiff/ |
Titled "'schemadiff: in-memory schema diffing, normalization, validation and manipulation'"
Randomly scheduled for April 15 (which is on a weekend) -- we'll change that per blog release schedule.