expression,parser: add support for INTERVAL function #2370
Conversation
|
Cool staff. |
|
"INTERVAL" is a reserved keyword of MySQL. See http://dev.mysql.com/doc/refman/5.7/en/keywords.html Please add it to the ReservedKeyWord rule list https://github.com/pingcap/tidb/blob/master/parser/parser.y#L2069 and add a test here: https://github.com/pingcap/tidb/blob/master/parser/parser_test.go#L60 |
|
I see It's already been in the Is there any thing else I should do? @shenli |
|
@zyguan Yes, you are right. |
| v2, _ := d2.ToFloat64(sc) | ||
| return v1 < v2 | ||
| } | ||
| v1, _ := d1.ToInt64(sc) |
There was a problem hiding this comment.
I guess the actual behavior is use float for comparison:
mysql> SELECT INTERVAL(23, 1.7, 15.3, 23.1, 30, 44, 200);
+--------------------------------------------+
| INTERVAL(23, 1.7, 15.3, 23.1, 30, 44, 200) |
+--------------------------------------------+
| 2 |
+--------------------------------------------+
1 row in set (0.00 sec)
| for _, t := range []struct { | ||
| args []types.Datum | ||
| ret int64 | ||
| }{ |
There was a problem hiding this comment.
add test case:
23, 1, 23, 23, 23, 30, 44, 200 // same integer appear many times
23, 1.7, 15.3, 23.1, 30, 44, 200 // float
| sc := ctx.GetSessionVars().StmtCtx | ||
|
|
||
| lt := func(d1, d2 types.Datum) bool { | ||
| if d1.Kind() == types.KindString || d2.Kind() == types.KindString { |
There was a problem hiding this comment.
I don't think we need take much consideration for this.
user will get wrong result because their input is illegal.
when the N1...Nn is not sorted, the result is also undefined since we use binary search.
There was a problem hiding this comment.
How about this: if there is any non-integer datum, we compare them as float ?
There was a problem hiding this comment.
That's more reasonable.
| @@ -2448,6 +2451,7 @@ FunctionNameConflict: | |||
| | "UTC_DATE" | |||
| | "CURRENT_DATE" | |||
| | "VERSION" | |||
| | "INTERVAL" %prec lowerThanIntervalKeyword | |||
There was a problem hiding this comment.
Is it ok to add "interval" to https://github.com/pingcap/tidb/pull/2370/files#diff-27c45ca411f005e1b8796b12fb53e26cL60 ?
There was a problem hiding this comment.
Sorry, I think I do not catch your idea, could you please explain further?
There was a problem hiding this comment.
INTERVAL is a reserved keyword in MySQL, for reserved keyword, you need add it here , here and test it here
This is because reserved keyword can't be used as table name, database name and so on, not the same as normal identifier.
There was a problem hiding this comment.
ignore this comment, INTERVAL is already added @zyguan
|
@tiancaiamao PTAL |
| if d1.Kind() == types.KindUint64 && d2.Kind() == types.KindInt64 { | ||
| return d2.GetInt64() > 0 && d1.GetUint64() < d2.GetUint64() | ||
| } | ||
| v1, _ := d1.ToFloat64(sc) |
There was a problem hiding this comment.
Ignore error here means if input is wrong, it always give a result.
There was a problem hiding this comment.
Yes, it is. Maybe we need check for some special cases, but I'm still unable to make INTERVAL produce an error in mysql.
BTW, can we close #112 now?
|
LGTM |
This is the last builtin function wanted by #112 :)