fix: get schema was not parsing statements with autoincrement #430
Conversation
…tdeploy] Signed-off-by: Bruno Calza <[email protected]>
| if strings.Contains(strings.ToLower(createStmt), "autoincrement") { | ||
| createStmt = strings.Replace(createStmt, "autoincrement", "", -1) | ||
| } |
There was a problem hiding this comment.
Uhm, OK, looks fine, but I'm a bit worried we can replace any other stuff with autoincrement that shouldn't be replaced. For example, if a column name is autoincrement, something bad would happen.
Of course, this is a very weird border case, but still possible.
I can suggest two options:
1- Simply accept that risk, since it would be very weird if this happened; and wait for a better overall solution, probably related to the other chat about generating "spec compatible" strings.
2- We can do a similar strategy as with strict, looking for a more concrete position to replace. For example, maybe look for primary key autoincrement (not sure if that would be correct, but you get the idea), and only replace it there. That will probably get rid of this problem while still being a simple-ish solution.
I'm fine with either.
There was a problem hiding this comment.
Good point! I just checked and we're not allowing AUTOINCREMENT as a column name (and in any other place), even if we enclose it with brackets or similar. So we should be safe.
Summary
When getting the schema of a table we need to parse the statement that is stored in SQLite. Sometimes that statement may come with an
AUTOINCREMENT(because of the protocol rules that add theAUTOINCREMENTkeyword automatically when anINTEGER PRIMARY KEYis found). However, theAUTOINCREMENTkeyword is not parsable. So we're getting an error.Context
Closes #428
Implementation overview
Simply remove the
AUTOINCREMENTbefore parsing.I don't think removing keywords that are not parsable such as
STRICTandAUTOINCREMENTis a good long-term approach. But this is good enough to solve thebug.This is also related to tablelandnetwork/go-sqlparser#41. It is kind of weird that the
Stringmethod produces and string that is not compliant with the Tableland SQL. But that's more a matter of definition. Up until now, we've been using theStringmethod as a way to get the SQL statement that will be executed inSQLite. I'll leave this discussion for the future.