-
Notifications
You must be signed in to change notification settings - Fork 411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial TiDB Support #724
Initial TiDB Support #724
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I will take some time to review it properly, but I will most likely merge it.
I see you have taken MySQL as a base for implementing TiDB support. It would be nice if you could describe in a few words what are the differences from MySQL that you have implemented.
If you would like to help even more, you could go and update the wiki pages with information about TiDB.
I personally have zero knowledge of this dialect. Actually, up to this day I didn't even know it existed.
If you want to get some hands on experience, there are a few options. For production deployments you'll need multiple components, but for testing this isn't required, which makes this a lot easier. Option 1: Docker
This runs TiDB on port 4000. Connect with Option 2: TiUP
This runs a full cluster locally on your machine, including dashboards etc. See also https://tiup.io Option 3: TiDB Cloud https://tidbcloud.com has a free tier. Option 4: Build and run TiDB locally.
This runs TiDB on port 4000. Connect with TiDB implements the MySQL protocol and syntax. See https://docs.pingcap.com/tidb/stable/mysql-compatibility for details on compatibility. There are some TiDB specific functions: https://docs.pingcap.com/tidb/stable/tidb-functions and some other things like support for sequences which are not in MySQL. Note that MariaDB also has sequences support. |
https://github.com/pingcap/tidb/blob/master/pkg/parser/parser.y might be a useful resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the effort you've put into this.
'ACCOUNT', | ||
'ACTION', | ||
'ADVISE', | ||
'AFTER', | ||
'AGAINST', | ||
'AGO', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you've included all the non-reserved keywords. I will remove these as the goal of this file is to only list the reserved ones.
It's a tradeoff that this formatter makes. Listing all possible keywords here will ensure they all get uppercased, but it will also cause a lot of column names to be mistaken for keywords and uppercased as well.
'FLOAT', // (R) | ||
'FLOAT4', // (R) | ||
'FLOAT8', // (R) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you've also included all data type names to list of keywords.
While these are keywords, they've been excluded from this list on purpose and are listed in this same file separately.
I'll remove these.
@@ -0,0 +1,295 @@ | |||
export const functions: string[] = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason you have excluded aggregate functions like count()
, avg()
, etc. I'm pretty confident that TiDB supports these (like all other SQL dialects I'm aware of).
You've also excluded cast()
.
All of these are probably treated internally special syntax and therefore not listed under functions. But for the purposes of the formatter, we want to format them all similarly.
If you had run the test-suite you would have seen lots of tests failing because of these missing functions.
I'll add these.
Closes #723