Skip to content
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

feat(expr): support cast from number to varchar #2522

Merged
merged 2 commits into from
May 16, 2022

Conversation

xiaoyong-z
Copy link
Contributor

@xiaoyong-z xiaoyong-z commented May 14, 2022

Signed-off-by: xyz [email protected]

What's changed and what's your intention?

add support from number[int16/int32/int64/float32/float64/decimal] to varchar using to_string method

PLEASE DO NOT LEAVE THIS EMPTY !!!

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

#112

@xiaoyong-z xiaoyong-z force-pushed the number-to-varchar branch from cfc0b27 to dca3e30 Compare May 14, 2022 15:35
@xiaoyong-z xiaoyong-z changed the title feat: support cast from number to varchar feat(expr): support cast from number to varchar May 14, 2022
Copy link
Member

@fuyufjh fuyufjh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@BugenZhao BugenZhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! It seems we can also replace bool_to_str with general_to_string now?

src/expr/src/expr/expr_unary.rs Outdated Show resolved Hide resolved
@TennyZhuang
Copy link
Contributor

TennyZhuang commented May 16, 2022

clippy failed

error: casting float literal to `f32` is unnecessary
[452](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:452)
   --> src/expr/src/vector_op/cast.rs:277:38
[453](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:453)
    |
[454](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:454)
277 |         assert_eq!(general_to_string(32.12 as f32).unwrap(), "32.12");
[455](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:455)
    |                                      ^^^^^^^^^^^^ help: try: `32.12_f32`
[456](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:456)
    |
[457](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:457)
    = note: `-D clippy::unnecessary-cast` implied by `-D warnings`
[458](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:458)
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
[459](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:459)

[460](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:460)
error: casting float literal to `f32` is unnecessary
[461](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:461)
   --> src/expr/src/vector_op/cast.rs:278:38
[462](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:462)
    |
[463](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:463)
278 |         assert_eq!(general_to_string(-32.14 as f32).unwrap(), "-32.14");
[464](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:464)
    |                                      ^^^^^^^^^^^^^ help: try: `-32.14_f32`
[465](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:465)
    |
[466](https://github.com/singularity-data/risingwave/runs/6445857930?check_suite_focus=true#step:9:466)
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

@xiangjinwu xiangjinwu mentioned this pull request May 16, 2022
3 tasks
@xiaoyong-z xiaoyong-z force-pushed the number-to-varchar branch from 51b8b5d to dd427bd Compare May 16, 2022 14:30
Copy link
Member

@BugenZhao BugenZhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #2522 (8692480) into main (6b7d5b7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2522      +/-   ##
==========================================
+ Coverage   72.17%   72.18%   +0.01%     
==========================================
  Files         674      674              
  Lines       87831    87853      +22     
==========================================
+ Hits        63395    63421      +26     
+ Misses      24436    24432       -4     
Flag Coverage Δ
rust 72.18% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/expr/src/expr/expr_unary.rs 75.97% <ø> (ø)
src/expr/src/vector_op/cast.rs 59.88% <100.00%> (+9.21%) ⬆️
src/meta/src/model/barrier.rs 78.57% <0.00%> (-3.58%) ⬇️
src/meta/src/stream/stream_manager.rs 63.86% <0.00%> (-0.15%) ⬇️
src/common/src/types/decimal.rs 82.25% <0.00%> (+0.26%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@TennyZhuang TennyZhuang merged commit 8d0d69b into risingwavelabs:main May 16, 2022
@xiaoyong-z xiaoyong-z deleted the number-to-varchar branch May 31, 2022 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants