Add support for SET autocommit=ON/OFF

[kuba--]

Signed-off-by: kuba-- kuba@sourced.tech

Some drivers (notably Python's mysql-connector) set some variables by using ON and OFF rather than using 1 and 0, e.g:

• SET @@session.autocommit=OFF
• SET @@session.autocommit=ON

So far, we have support for ON but OFF was missed. That's why OFF statements expression (sqlparser.Expr) has incorrect type (*sqlparser.ColName, instead of `*sqlparser.SQLVal).

This PR adds support also for OFF value for SET expressions.

[sougou]

Nice work 👍

• Looks like we missed the test for ON (consequently OFF) in parse_test.go. Can you add the two cases?
• The goyacc version you used doesn't agree with the one used by Travis. So, that test is failing. Can you update (probably the latest version) and regenerate sql.go?

[kuba--]

@sougou - I've added tests for parser.
Regarding yacc. I have the latest version (by running go get -u github.com/golang/tools/cmd/goyacc it downloaded the latest master: c756801b0141e5d8f5f43e38d9962117e034e565).

So it's hard to say why it generates different sql.go. Maybe it uses native yacc (mine is:

yacc --version
bison (GNU Bison) 2.3
Written by Robert Corbett and Richard Stallman.

)

Just for info. I've just ran make (inside go/vt/sqlparser directory) to generate sql.go

[sougou]

In that case, we may have to update goyacc on the vitess side. I'll push this through if that's the only test failing.

[kuba--]

ok, cool. btw. I've modified sql.go manually so the latest build passed ;)

[demmer]

@sougou We're just catching up on upstream changes and I noticed this PR adds OFF as a keyword.

This seems like a different behavior from mysql upstream (unlike ON) so I'm a bit curious if this is an issue. Specifically:

mysql> select off from mytable;
ERROR 1054 (42S22): Unknown column 'off' in 'field list'
mysql> select on from mytable;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'on from mytable' at line 1

[kuba--]

@demmer - I think, if we move OFF from keywords to non-keywords in sql.y it may work for all cases.
My case was mainly SET autocommit=OFF, where ON/OFF stands for 1/0

[demmer]

Yeah -- I understand why this was added.. I guess my question is whether OFF needed to be a token or if we could evolve the implementation of the SET command to accept a ColIdent.

[sougou]

I believe non-reserved keyword and ColIdent are somewhat equivalent. Strangely, according to https://dev.mysql.com/doc/refman/5.7/en/keywords.html, ON is a keyword but OFF is not.
Personally, I think it will look cleaner if OFF was on par with ON. So, voting for non-reserved.

Could be convinced otherwise. No strong feeling in particular.

[demmer]

I think non-reserved makes more sense to me.

Actually I meant the complete opposite....

I don't think we should have keywords which aren't in mysql (and aren't for some vitess-specific use case) even if they are non-reserved.

But I could be convinced otherwise as well.

[yuananf]

#3896
I remember I added the support for SET autocommit=ON/OFF