Repository navigation
mysql alter add inside of schema file fails #251
Description
Say a schema file contains
alter table people add column is_cool tinyint;
update people set is_cool = 0 where 1 = 1;
sqlc will error out with:
Failed to parse UPDATE query: Failed to determine type of a parameter's column: Column [is_cool] not found in table [people]
Since update commands (and similar) don't actually change the schema, we could get away with dropping them completely. But perhaps all we're missing is updating the right bits of memory between query parses?
(An unedited version of this was found in a codebase that's in production.)
Activity
[-]mysql update inside of schema queries fails[/-][+]mysql update inside of schema file fails[/+]jmhodges commentedon Jan 10, 2020
(The third line was
in case you're wondering why this wasn't a
not nullsituation.)cmoog commentedon Jan 12, 2020
The problem here isn't the
updatestatement but the fact that we currently don't parse thealterstatement. I can't seem to find wheresqlparserholds information foralterqueries...Am I missing something obvious here?? Looks like the only things it parses are the
ActionandTableName.jmhodges commentedon Jan 13, 2020
Yeah, this def seems to be a missing feature in sqlparser itself.
This section of their current sql.y is useful to understand:
https://github.com/vitessio/vitess/blob/3670303cca67e6833393a80667b45491b866b0d1/go/vt/sqlparser/sql.y#L1291-L1294
Those lines drop everything after the
ADDpart of anALTER sometable ADDstatement, and create asqlparser.DDLwith just the Action string set toALTERand theTablestring set to thetable_nameit parses out.That file, at minimum, needs to be patched to parse out the types at set them on that
sqlparser.DDLlike similar files do.[-]mysql update inside of schema file fails[/-][+]mysql alter add inside of schema file fails[/+]jmhodges commentedon Jan 13, 2020
I've changed the title of this issue. (I was hesitant about changing the summary. @kyleconroy would you prefer a new ticket?)
The https://github.com/pingcap/parser mentioned by another user in #150 seems to support this, but I'm not sure if we should switch. I'm not sure what the decision making was on our selection here (and don't have a horse in this race).
@clintberry you mentioned vitess being interested in sqlc. How amenable would vitess be to patches for fuller ALTER support (and are there already some in progress)?
jmhodges commentedon Jan 13, 2020
Okay, I heard from Andrés Taylor that vitess is down for us to add more ALTER support to sqlparser. He says they're consciously trying to increase the SQL they support and, if we get stuck, that he's up to help us out.
I've made a tracking ticket in their repo vitessio/vitess#5705
kyleconroy commentedon Aug 25, 2020
The new
mysql:betaengine parses this code correctly and does not error.