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

dag/expr: builtin_cast-III #2176

Merged
merged 98 commits into from Aug 28, 2017

Conversation

Projects
None yet
4 participants
@AndreMouche
Member

AndreMouche commented Aug 15, 2017

Hi,
This PR implement the following cast functions:

  1. cast_time_as_decimal
  2. cast_duration_as_decimal
  3. cast_json_as_decimal
  4. cast_decimal_as_decimal
  5. cast_real_as_decimal
  6. cast_int_as_str
  7. cast_real_as_str
  8. cast_decimal_as_str
  9. cast_str_as_str
  10. cast_time_as_str
  11. cast_duration_as_str
  12. cast_json_as_str
  13. cast_*_as_time
  14. cast_*_as_duration
  15. cast_*_as_json

And it's based on PR #2172
You'd better reviewer after the base PR been merged.

@BusyJay @andelf @hicqu @XuHuaiyu PTAL

AndreMouche added some commits Aug 9, 2017

@@ -128,6 +128,24 @@ impl Time {
self.tp
}
pub fn set_tp(&mut self, tp: u8) -> Result<()> {
if self.tp != tp && tp == types::DATE {

This comment has been minimized.

@BusyJay

BusyJay Aug 22, 2017

Contributor

How to deal with datetime and timestamp?

@BusyJay

BusyJay Aug 22, 2017

Contributor

How to deal with datetime and timestamp?

This comment has been minimized.

@AndreMouche

AndreMouche Aug 23, 2017

Member

we return error when convert datetime/date to timestamp @BusyJay

@AndreMouche

AndreMouche Aug 23, 2017

Member

we return error when convert datetime/date to timestamp @BusyJay

This comment has been minimized.

@BusyJay

BusyJay Aug 23, 2017

Contributor

How about convert timestamp to datetime? For example, a timestamp is 2017/08/20 12:00:00 UTC, and it needs to be converted to datetime in +08:00, what's the expected result?

@BusyJay

BusyJay Aug 23, 2017

Contributor

How about convert timestamp to datetime? For example, a timestamp is 2017/08/20 12:00:00 UTC, and it needs to be converted to datetime in +08:00, what's the expected result?

This comment has been minimized.

@AndreMouche

AndreMouche Aug 23, 2017

Member

It would return datetime in +08:00 here @BusyJay

@AndreMouche

AndreMouche Aug 23, 2017

Member

It would return datetime in +08:00 here @BusyJay

Show outdated Hide outdated src/coprocessor/codec/mysql/time.rs
Show outdated Hide outdated src/coprocessor/codec/mysql/time.rs
Show outdated Hide outdated src/coprocessor/codec/mysql/time.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/codec/mysql/time.rs
Show outdated Hide outdated src/coprocessor/codec/mysql/time.rs
Show outdated Hide outdated src/coprocessor/dag/expr/mod.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/codec/convert.rs
Show outdated Hide outdated src/coprocessor/codec/mysql/time.rs
Show outdated Hide outdated src/coprocessor/codec/mysql/time.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
row: &'a [Datum],
) -> Result<Option<Cow<'a, Json>>> {
let val = try_opt!(self.children[0].eval_decimal(ctx, row));
let val = try!(val.as_f64());

This comment has been minimized.

@BusyJay

BusyJay Aug 24, 2017

Contributor

Can val be inf?

@BusyJay

BusyJay Aug 24, 2017

Contributor

Can val be inf?

Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
Show outdated Hide outdated src/coprocessor/dag/expr/builtin_cast.rs
@@ -15,7 +15,7 @@ use super::Result;
use util::escape;
/// `UN_SPECIFIED_FSP` is the unspecified fractional seconds part.
const UN_SPECIFIED_FSP: i8 = -1;
pub const UN_SPECIFIED_FSP: i8 = -1;

This comment has been minimized.

@hicqu

hicqu Aug 24, 2017

Contributor

maybe should be UNSPECIFIED_FSP?

@hicqu

hicqu Aug 24, 2017

Contributor

maybe should be UNSPECIFIED_FSP?

This comment has been minimized.

@AndreMouche

AndreMouche Aug 24, 2017

Member

I think both are ok here. Since this PR is too large, maybe we could change it in an independent PR someday? @hicqu

@AndreMouche

AndreMouche Aug 24, 2017

Member

I think both are ok here. Since this PR is too large, maybe we could change it in an independent PR someday? @hicqu

@AndreMouche

This comment has been minimized.

Show comment
Hide comment
@AndreMouche
Member

AndreMouche commented Aug 28, 2017

@BusyJay

This comment has been minimized.

Show comment
Hide comment
@BusyJay

BusyJay Aug 28, 2017

Contributor

LGTM

Contributor

BusyJay commented Aug 28, 2017

LGTM

@choleraehyq

This comment has been minimized.

Show comment
Hide comment
@choleraehyq

choleraehyq Aug 28, 2017

LGTM
Please resolve the conflict.

choleraehyq commented Aug 28, 2017

LGTM
Please resolve the conflict.

@hicqu

hicqu approved these changes Aug 28, 2017

@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Aug 28, 2017

Contributor

LGTM.

Contributor

hicqu commented Aug 28, 2017

LGTM.

@AndreMouche AndreMouche merged commit ba174ad into master Aug 28, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@AndreMouche AndreMouche deleted the shirly/eval_cast_3 branch Aug 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment