Support push function concat, concat_ws, datediff, year, day, unix_timestamp down to TiFlash.
#1588
Conversation
|
Maybe add some unit tests or integration tests(port from TiDB)? |
| @@ -1303,13 +1303,11 @@ class FunctionTiDBTimestampDiff : public IFunction | |||
| throw Exception("First argument for function " + getName() + " (unit) must be String", | |||
| ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); | |||
|
|
|||
| if(!checkDataType<DataTypeMyDateTime>(removeNullable(arguments[1]).get()) && | |||
| !checkDataType<DataTypeMyDate>(removeNullable(arguments[1]).get())) | |||
| if(!removeNullable(arguments[1])->isDateOrDateTime()) | |||
There was a problem hiding this comment.
Why change this? DataTypeDate, DataTypeDatetime is not supported yet.
There was a problem hiding this comment.
can we assume that this TiDB Function will never accept the non-TiDB datatype?
There was a problem hiding this comment.
I thought we won't build DataTypeDate or DataTypeDateTime arguments for this function anyway?
There was a problem hiding this comment.
But we do not delete DataTypeDate and DataTypeDateTime, I think it's better to throw error explicitly if the function does not support DataTypeDate and DataTypeDateTime
| throw Exception("Second argument for function " + getName() + " must be MyDate or MyDateTime", | ||
| ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); | ||
|
|
||
| if(!checkDataType<DataTypeMyDateTime>(removeNullable(arguments[2]).get()) && | ||
| !checkDataType<DataTypeMyDate>(removeNullable(arguments[2]).get())) | ||
| if(!removeNullable(arguments[2])->isDateOrDateTime()) |
|
/run-all-tests |
| @@ -1303,13 +1303,11 @@ class FunctionTiDBTimestampDiff : public IFunction | |||
| throw Exception("First argument for function " + getName() + " (unit) must be String", | |||
| ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); | |||
|
|
|||
| if(!checkDataType<DataTypeMyDateTime>(removeNullable(arguments[1]).get()) && | |||
| !checkDataType<DataTypeMyDate>(removeNullable(arguments[1]).get())) | |||
| if(!removeNullable(arguments[1])->isDateOrDateTime()) | |||
There was a problem hiding this comment.
But we do not delete DataTypeDate and DataTypeDateTime, I think it's better to throw error explicitly if the function does not support DataTypeDate and DataTypeDateTime
LittleFall
changed the title
concat, concat_ws, datediff, year, day, unix_timestamp down to TiFlash.
LittleFall
added
type/new-feature
What problem does this PR solve?
Issue Number: partially #1480
What is changed and how it works?
already supported:
opened
supported
Related changes
pingcap/docs/pingcap/docs-cn: need update push down docsCheck List
Tests
Release note