-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Parse MySQL column default value #110
Conversation
src/mysql/parser/column.rs
Outdated
default[1..(default.len() - 1)].into(), | ||
)) | ||
} else if default == "NULL" { | ||
None |
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 think 'default NULL' has a different meaning to 'no default'.
Seems like we should add a new variant ColumnDefault::Null
.
In practice, the default value could be 0
or ''
instead of NULL
.
@@ -264,7 +264,25 @@ pub fn parse_column_default(column_default: Option<String>) -> Option<ColumnDefa | |||
match column_default { | |||
Some(default) => { | |||
if !default.is_empty() { |
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 just read https://dev.mysql.com/doc/refman/8.0/en/data-type-defaults.html again,
I think the logic should be as follows:
- parse the
EXTRA
column; if it containsDEFAULT_GENERATED
, then theCOLUMN_DEFAULT
should be regarded as an expression - if the default is an expression, parse it as a
Expr
and recognize the keywordsCURRENT_TIMESTAMP
, otherwiseCustom
- if the default is not an expression, then it's a literal; then we should
a. if it is numeric-like then regard it asNumber
b. otherwise regard it asString
- if it starts with a
(
regard it as an expression - if it is numeric-like then regard it as
Number
. not sure whether it's worth the effort to classify intoint
andreal/decimal
- finally, consider it a
String
It's really a legacy that MySQL does not quote a literal string. It's given as hello
instead of 'hello'
.
Edit: I was wrong; apparently COLUMNS
table does not give a ()
for expressions either.
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.
Hmmm... no. This logic doesn't work on every MySQL / Mariadb version
|
May be we simply regard
and this https://mariadb.com/kb/en/information-schema-columns-table/ https://jira.mariadb.org/browse/MDEV-13132
|
Sadly to implement this correctly we have to perform some version check: MySQL 5:
MySQL 8:
MariaDB < 10.2.1: MariaDB >= 10.2.1:
|
Hey @tyt2y3, was a success. Please proofread the db specific parsing part for me :) |
src/mysql/parser/column.rs
Outdated
if is_date_time_col && default == "CURRENT_DATE" { | ||
ColumnDefault::CurrentDate | ||
} else if is_date_time_col && default == "CURRENT_TIME" { | ||
ColumnDefault::CurrentTime |
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.
Looks pretty promising! My only question is, are you sure current_date/time
is 'as special as' current_timestamp
?
Judging by the docs, it only mentioned CURRENT_TIMESTAMP:
The exception is that, for TIMESTAMP and DATETIME columns, you can specify the CURRENT_TIMESTAMP function as the default, without enclosing parentheses. See Section 11.2.5, “Automatic Initialization and Updating for TIMESTAMP and DATETIME”.
It didn't say CURRENT_DATE can be used for DATE, etc.
This means, for example, that you cannot set the default for a date column to be the value of a function such as NOW() or CURRENT_DATE.
I belive it does not work under MySQL 5.x
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 just double checked, yes, you're right. current_date/time
cannot be used as default value / expression.
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.
Removed CurrentDate and CurrentTime fca3ee5
src/mysql/parser/column.rs
Outdated
} | ||
|
||
pub fn parse_mysql_8_default(default: String, extra: &str) -> ColumnDefault { | ||
let is_expression = extra == "DEFAULT_GENERATED"; |
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.
The extra can have more string parts, not just DEFAULT_GENERATED
, a contains()
will be safer.
The right way would be to split the string by ' '
and then Vec::contains.
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.
} else if let Ok(double) = default.parse::<f64>() { | ||
ColumnDefault::Double(double) | ||
} else { | ||
ColumnDefault::String(default) |
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.
in this case the tail case should be split:
- is_expression = true -> ColumnDefault::CustomExpr
- is_expression = false -> ColumnDefault::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.
src/mysql/parser/column.rs
Outdated
} else if default == "NULL" { | ||
ColumnDefault::Null | ||
} else { | ||
ColumnDefault::String(default) |
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 believe the final case is a CustomExpr;
https://mariadb.com/kb/en/create-table/#default-column-option
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.
Done |
I just checked on my machine, MySQL 8 |
Wait... I am re-reading this
It means for the versions >= 10.2.1 and < 10.2.7 is a void, and there is no way to properly parse the default. |
🎉 Released In 0.12.0 🎉Thank you everyone for the contribution! |
PR Info
Bug Fixes