Add basic vector support for MySQL 9.x #16464
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
vitess-bot
bot
added
NeedsBackportReason
| @@ -8638,6 +8644,7 @@ non_reserved_keyword: | |||
| | VARIABLES | |||
| | VARIANCE %prec FUNCTION_CALL_NON_KEYWORD | |||
| | VCPU | |||
| | VECTOR | |||
There was a problem hiding this comment.
This follows how the new VECTOR type works in MySQL 9.0. It's not a reserved keyword, which means that for example using a column named vector that is not escaped in a SELECT vector FROM t1 still works.
This also means that the backwards compatibility here is guaranteed, since no working query today would break with this change.
| | VECTOR length_opt | ||
| { | ||
| $$ = &ColumnType{Type: string($1), Length: $2} | ||
| } |
There was a problem hiding this comment.
This is the new column type definition that is possible with VECTOR. We can add it safely here to the parser, since even if we can parse it here, the underlying MySQL will then error with a syntax error. That is the same behavior as today regardless, since if we have non-properly parsed DDL, we already pass it down as a best effort to MySQL anyway.
So a syntax error today that the user sees already comes from MySQL and that doesn't change. In general also, our Vitess parser is more lax than MySQL itself. It's reverse engineered from MySQL and in case of doubt we always opt for accepting a bit more than MySQL would if we can't easily seem to match its behavior. That way we don't block valid queries and invalid queries are tried but then throw a syntax error regardless from MySQL itself.
There was a problem hiding this comment.
Nit: in some places we use ParseStrictDDL(), such as in ApplySchema or in schemadiff, in which case we validate the syntax before attempting to apply in MySQL. But since the new syntax is more permissive, that's generally OK.
If a user were to try creating a table with vector columns via ApplySchema and on MySQL 8.0, the behavior would be somewhat different - instead of failing at the ApplySchema level, it would fail in MySQL.
This is a slight change of behavior because normally, if you apply 10 changes in one ApplySchema, and one of them has a syntax error, we fail all 10 before even making contact with MySQL. Now, we would pass all 10 into MySQL, some changes will apply, then the vector one would fail. This is still acceptable IMHO, but noting a change of behavior.
There was a problem hiding this comment.
This could be updated later, once the capabilities code knows about vector?
dbussink
added
Type: Enhancement
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16464 +/- ##
==========================================
- Coverage 68.65% 68.63% -0.02%
==========================================
Files 1550 1550
Lines 199412 199443 +31
==========================================
- Hits 136900 136889 -11
- Misses 62512 62554 +42 ☔ View full report in Codecov by Sentry. |
|
CI caught actually a good case here specifically for what we're trying to do here! I also added the type to the JDBC driver 😄. |
dbussink
requested review from
deepthi,
harshit-gangal,
mattlord,
systay,
shlomi-noach,
rohit-nayak-ps,
timvaillancourt,
frouioui,
GuptaManan100 and
vmg
as code owners
| | VECTOR length_opt | ||
| { | ||
| $$ = &ColumnType{Type: string($1), Length: $2} | ||
| } |
There was a problem hiding this comment.
Nit: in some places we use ParseStrictDDL(), such as in ApplySchema or in schemadiff, in which case we validate the syntax before attempting to apply in MySQL. But since the new syntax is more permissive, that's generally OK.
If a user were to try creating a table with vector columns via ApplySchema and on MySQL 8.0, the behavior would be somewhat different - instead of failing at the ApplySchema level, it would fail in MySQL.
This is a slight change of behavior because normally, if you apply 10 changes in one ApplySchema, and one of them has a syntax error, we fail all 10 before even making contact with MySQL. Now, we would pass all 10 into MySQL, some changes will apply, then the vector one would fail. This is still acceptable IMHO, but noting a change of behavior.
dbussink
force-pushed
the
vector-support
branch
2 times, most recently
from
4302481 to
df2d659
Compare
Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
This adds the basic type support for the new MySQL 9.0 vector type. See also #16463
Related Issue(s)
Fixes #16463
Checklist