mysql alter add inside of schema file fails

[jmhodges]

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.)

[jmhodges]

(The third line was

alter table subscriptions modify include_subdomains tinyint not null;

in case you're wondering why this wasn't a not null situation.)

[cmoog]

The problem here isn't the update statement but the fact that we currently don't parse the alter statement. I can't seem to find where sqlparser holds information for alter queries...

Am I missing something obvious here?? Looks like the only things it parses are the Action and TableName.

[jmhodges]

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 ADD part of an ALTER sometable ADD statement, and create a sqlparser.DDL with just the Action string set to ALTER and the Table string set to the table_name it parses out.

That file, at minimum, needs to be patched to parse out the types at set them on that sqlparser.DDL like similar files do.

[jmhodges]

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]

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]

The new mysql:beta engine parses this code correctly and does not error.