Repository navigation
MySQL support #150
Description
Activity
There's a high-quality open source MySQL parser included in the vitess project (https://godoc.org/github.com/youtube/vitess/go/vt/sqlparser#BuildParsedQuery). I'll take a look into using that.
Reacted by Jeff Hodges, Christopher Taylor, Drew Repasky, Hayato Tomoda, Clint Berry, Yunchi Luo, Jesse Dubay and koodimetsaWe actually are looking at using this for vitess in particular. Super excited you are at least looking at it.
TiDB also has a pretty good standalone MySQL parser worth taking a look at: https://github.com/pingcap/parser
I've been using sqlc in a hobby project and really love it. Would love to see MySQL support so I can use it at work too.
@kyleconroy
This project is super cool and basically the best solution I've come across for adding type safety and code generation to the data access layer. I'm in the process of adding MySQL support on a fork branch here.
A few things to note:- much of the generator logic is specific to the
pgparser. On my branch, I've attempted to abstract the Go generator logic to be parser-agnostic. To further clean and decouple the components, I think it would be wise to separategeninto its own package. - how do you envision the user specifying their desired parser? From the CLI or in the config file?
The status of my branch is still very rough and lacking many implementations such as
- settings and overrides
- file reading and writing
- imports
- a few parameter conditions that fail
It's basically only unit tests validating that basic parsing and generation is functional.
Reacted by Jeff Hodges- much of the generator logic is specific to the
@jmhodges @clintberry I just merged experimental MySQL support. It's not production ready, but should work for simple queries and schema. Please try it out and report any bugs.
Wilco! Found some. Will make separate tickets (I can bring them back here and close the ones I make if you'd like)
So i was chatting with @jmhodges earlier, I'd like to recommend against vitess for a couple reasons, after talking through with systay (Andres) (maintainer of the vitess query/parsing code)
- vitess's parser intentionally doesn't support all of mysql syntax. The parser only supports what vitess supports, intentionally failing on things that vitess does not, so the user sees an error instead of getting their statement ignored
- if we want to add features to the parser, we would need to first add a second parsing pass that would throw on unsupported vitess features. Otherwise we would cause problems for Vitess. This is probably simple, but enumerating the features and the actual work might be tedious
- there is an unknown number of features that the vitess parser doesn't support from the mysql syntax, and the method for discovering incompatibilities will probably be by writing test cases and parsing a ton of code examples. The burden is already being felt by @jmhodges in implementing alter support mysql alter add inside of schema file fails #251
- while planetscale (the company that builds vitess) is in the process of expanding its mysql compatibility, extending the sql parser isn't quite on their list of priorities yet, so if we want to use their parser, the work is on us
Meanwhile, i was poking around tidb's parser, and according to this they should be fairly complete: pingcap/tidb#11486. This seems like a happier path. I'm pretty noob to compilers and AST so I could be totally wrong too.
I know @jmhodges and @jmhodges have done a ton of work around the current parser that uses Vitess, so I'm curious about your thoughts. No idea how much work would need to be redone to switch parsers. I personally am just interested in getting the best possible MySQL support so whatever gets us there is best! And I'd be happy to contribute if you think this is a good idea, I'm just a compilers noob so it might take me a while.
Meanwhile, i was poking around tidb's parser, and according to this they should be fairly complete: pingcap/tidb#11486. This seems like a happier path. I'm pretty noob to compilers and AST so I could be totally wrong too.
Upon closer examination, this does look like a viable alternative to
vitess. I'll take a closer look at the documentation and try to assess the work and how difficult a conversion would be.I personally am just interested in getting the best possible MySQL support so whatever gets us there is best!
Me too! I'll definitely defer to @jmhodges on the difficulty of adding features to
vitess, so I agree we should seriously consider migrating totidbif the conversion effort justifies it.After writing my first patch to sqlparser, I can see how we might have more work than doing a conversion to the tidb parser. I think it's someone to trying out the conversion is worth a shot.
(As a helper, do we have any contacts working on that project?)
I have contacts! Most of their company is based in China though and they currently are on the Chinese New Year holidays, till sometime next week.
Generally, I’ve found them super receptive and helpful.
Not sure if people here are familiar with the project, but I use go-queryset which supports MySQL and is similar in that it generates type-safe Go code from definitions. But the schema is defined with go structs. For simple schemas it is probably easier to make definitions with go structs, but for more complex schemas declaring them in SQL could be a lot nicer. From a first look, sqlc appears to generate more polished high-level query code, but it may be much less extensible than what go-queryset generates. I normally write my own higher-level wrappers around go-queryset functions (for example to just return a list like the list functions in this project do rather than having to pass in an array).
Both projects are MIT licensed, so I could see this project benefiting from using some of the extensible query code in go-queryset.Hello, TiDB developer here. This is a cool project! We would love to help on this issue.
One aim of the parser is to provide the best MySQL compatible parser to all the Golang projects.@shenli Thank you so much for commenting. I wanted to make sure we had buy-in from TiDB before we switched parsers. It sounds like both projects (vitess and TiDB) would like us to use the TiDB parser! Consider this an official sign-off on the switch.
but I use go-queryset ... But the schema is defined with go structs
@gregwebs sqlc is dedicated to being SQL-first. go-queryset looks cool, but I think we're set with the current direction of the project.
2 remaining items
Consider this an official sign-off on the switch.
Great to hear. I'll get working on the conversion.
Okay, one problem I've seen with a conversion to the TiDB parser API is that it does not allow for returning the byte locations of syntax errors and adding that functionality would break its backwards compatibility.
The possible parse errors are exposed as package-level variables (e.g.
ErrSyntax,ErrParse) and those would have to instead by dynamically created to add where in the sql statement that parsing failed.meta: (just want to clarify that vitess didn't recommend using the tidb parser. I recommended that after chatting through some details with a vitess maintainer, and gauged that it may be more work to use the Vitess parser than to switch. Vitess would love for us to contribute to their parser and get it to be more mysql compatible. But for the moment we would be the ones doing the work to make it so and not break their existing guarantees)
The possible parse errors are exposed as package-level variables (e.g.
ErrSyntax,ErrParse) and those would have to instead by dynamically created to add where in the sql statement that parsing failed.It looks to me errors are returned as
*terror.Errorwhich has file and line. Extending it to return byte locations could be tedious, but I don't think it would be backwards incompatible. (Also, I think the current behavior is to match MySQL, which only tells you the line number when you have syntax errors... Not that's useful lol)I don't see any place that the
file, orlinefield is set. The code below emitserror: line 1 column 7 near "asfdadf" , *errors.errorStringwhich is just a
fmt.Errorf, not a structured error type, it seems. Or did I get something wrong?package main import ( "fmt" "log" "github.com/pingcap/errors" "github.com/pingcap/parser" _ "github.com/pingcap/tidb/types/parser_driver" ) func main() { p := parser.New() _, _, err := p.Parse("asfdadf", "", "") if err != nil { log.Printf("error: %s, %T", errors.Unwrap(err), errors.Unwrap(err)) } fmt.Println("worked but def shouldn't have") }Which is honestly odd, because I'm not sure why that's not getting a specific error type at all? (Before the
Unwrap, it's anerrors.withStack). With all that error code in the parser, it seems like all that gets thrown away before being returned?I surely could be missing something, but I still don't see a good place to put this in. Maybe @cmoog is having better luck
Bah, okay, ErrSyntax is only returned if there's zero statements in a call to
ParseOneStmtand ErrParse isn't used at all.So, I was wrong about the backwards incompatibility!
We could expose this stuff! It looks like this error string is coming from
Scanner.Errorfmethod and that seems like the part that needs to return a structured error type we can pull back apart.I don't see any place that the
file, orlinefield is set. The code below emitserror: line 1 column 7 near "asfdadf" , *errors.errorStringwhich is just a
fmt.Errorf, not a structured error type, it seems. Or did I get something wrong?package main import ( "fmt" "log" "github.com/pingcap/errors" "github.com/pingcap/parser" _ "github.com/pingcap/tidb/types/parser_driver" ) func main() { p := parser.New() _, _, err := p.Parse("asfdadf", "", "") if err != nil { log.Printf("error: %s, %T", errors.Unwrap(err), errors.Unwrap(err)) } fmt.Println("worked but def shouldn't have") }Basically the
lineandcolare set here: https://github.com/pingcap/parser/blob/master/lexer.go#L812 . Thereaderhandles the raw SQL string.@mightyguava My first pass didn't go so well. That said, I should have an update by the end of this week or early next. If I can't get something together by then, I'd be all good with handing it off to another contributor.
Thanks for the hard work!
At long last, I can close this issue! MySQL support will be released in version 1.6.0, which is going out in the next few days. Thanks to everyone that's worked on MySQL support.
Reacted by Yunchi Luo, Charlie Moog, Greg Weber, koodimetsa and Jeff Hodges
Love this project idea! All of my tools are built on mysql 5.x and it'd be great to have that supported.