expression: fix unix_timestamp function which is not compatible with Mysql

[wjhuang2016]

What problem does this PR solve?

fix #9729

What is changed and how it works?

The implement of the original only considers the case of Constant. We need to consider the case such as select unix_timestamp(@1)

I found that it's hard to build a test for such sistuation, maybe some one could tell we how to add the test?

Check List

Tests

Code changes

Side effects

Related changes

[codecov]

Codecov Report

Merging #9751 into master will increase coverage by 0.0272%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master      #9751        +/-   ##
================================================
+ Coverage   77.2106%   77.2378%   +0.0272%     
================================================
  Files           405        405                
  Lines         81661      81662         +1     
================================================
+ Hits          63051      63074        +23     
+ Misses        13933      13919        -14     
+ Partials       4677       4669         -8

[eurekaka]

For unit test, you can reference cases in expression/builtin_time_test.go to mock input, or you can add tests using SQL, see also expression/integration_test.go.

[qw4990]

LGTM

[qw4990]

PTAL @lamxTyler @eurekaka

[zz-jason]

Consider this case: args[0] is a scalar function, but it refers to other columns. Directly calling args[0].EvalString() would cause a nil pointer deference panic because the input Row is empty in this case.

A simple SQL can be:

select unix_timestamp(concat(a+b)) from t;

[wjhuang2016]

You're right, I think that some scalar functions would panic. And the test passing surprises we.
Can we eval it here for scalar function like concat?

[zz-jason]

For general scalar function, I think we can not. We can specially handle the GetVar function in the non-prepare statements. However, it's better to check how mysql handle this scenario in their codebase.

[wjhuang2016]

For general scalar function, I think we can not. We can specially handle the GetVar function in the non-prepare statements. However, it's better to check how mysql handle this scenario in their codebase.

Agree with you, we have better check the code in MySQL. And I think GetVar is not the only function we need to handle.

[eurekaka]

An off-topic finding: concat seems to have bug currently

mysql> select concat(a+b) from t;
ERROR 1054 (42S22): Unknown column 'b' in 'field list'
mysql> select concat(a, b) from t;
ERROR 1054 (42S22): Unknown column 'b' in 'field list'
mysql> select unix_timestamp(concat(a+b)) from t;
ERROR 1054 (42S22): Unknown column 'b' in 'field list'

it reports error in parser.

[wjhuang2016]

@eurekaka
Do you forget the column b when create table t?

mysql> create table t(a int, b int);
Query OK, 0 rows affected (0.04 sec)

mysql> select concat(a+b) from t;
Empty set (0.00 sec)

mysql> select concat(a, b) from t;
Empty set (0.01 sec)

mysql> select unix_timestamp(concat(a+b)) from t;
Empty set (0.01 sec)

[eurekaka]

😭 My fault.

[qw4990]

PTAL @wjhuang2016

[wjhuang2016]

PTAL @wjhuang2016

This PR rely on a issue #10168, and it's complex to fix it.
I think it's better to set this PR as DO NOT MERGE until we find a good way to determine whether a getVar func is a ConstItem.

[qw4990]

Since there is no way to determine if an expression is const accurately, we close this issue temporarily.