coprocessor/dag/expr: add some un-pushed builtin UDFs(Interval) #3330
Conversation
|
Hi contributor, thanks for your PR. This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically. |
| Err(e) => return Err(e), | ||
| Ok(None) => target, | ||
| Ok(Some(v)) => v, | ||
| }; |
There was a problem hiding this comment.
Could we use self.children[mid].eval_int(ctx,row)?.unwrap_or(target); instead?
There was a problem hiding this comment.
That would be awesome, thank you! @AndreMouche
|
@breeswish PTAL |
bb7133
force-pushed
the
bb7133/new_cops_interval
branch
from
3c2f1fa
to
0ac71fa
Compare
| } | ||
|
|
||
| pub fn interval_real(&self, ctx: &mut EvalContext, row: &[Datum]) -> Result<Option<i64>> { | ||
| let target = match self.children[0].eval_real(ctx, row) { |
There was a problem hiding this comment.
might be written as:
let target = match self.children[0].eval_real(ctx, row)? {
None => ...
Some(v) => ...
};| Ok(Some(v)) => v, | ||
| }; | ||
|
|
||
| let mut left = 1; |
There was a problem hiding this comment.
Why our initial left is 1 while in TiDB i starts from 0?
There was a problem hiding this comment.
The code at TiDB side makes a slice that starts from 1 when passing args to binSearch:
The code here initialize left as 1 and return left - 1 as result, which should be the same as TiDB code. @breeswish
| @@ -202,6 +202,60 @@ impl ScalarFunc { | |||
| pub fn in_json(&self, ctx: &mut EvalContext, row: &[Datum]) -> Result<Option<i64>> { | |||
| do_in(self, |v| v.eval_json(ctx, row), |l, r| Ok(l.cmp(r))) | |||
| } | |||
|
|
|||
| pub fn interval_int(&self, ctx: &mut EvalContext, row: &[Datum]) -> Result<Option<i64>> { | |||
| let target = match self.children[0].eval_int(ctx, row) { | |||
bb7133
force-pushed
the
bb7133/new_cops_interval
branch
from
0ac71fa
to
cc78a78
Compare
|
Comments addressed, thank you @breeswish |
|
Hi contributor, thanks for your PR. This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically. |
|
@AndreMouche Please give an approval :) |
AndreMouche
added
sig/coprocessor
|
/run-integration-tests |
What have you changed? (mandatory)
implemented ScalarFuncSig::IntervalInt and ScalarFuncSig::IntervalReal
What are the type of the changes? (mandatory)
Improvement
How has this PR been tested? (mandatory)
added some UTs
Does this PR affect documentation (docs/docs-cn) update? (mandatory)
No
Does this PR affect tidb-ansible update? (mandatory)
No
Refer to a related PR or issue link (optional)
#3275