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(binder): align type cast-ability with pg #1759

Merged
merged 10 commits into from
Apr 12, 2022
Merged

Conversation

xiangjinwu
Copy link
Contributor

@xiangjinwu xiangjinwu commented Apr 11, 2022

What's changed and what's your intention?

  • cast_ok checks type cast-ability for all 13 types we support so far in a pg-compatible way (See build_cast_map and test_cast_ok for details). It is also aware of cast context.
  • Binder::find_compat -> least_restrictive
    • Better refactoring of the call site is possible, to work on a collection of types rather than 2. Will be handled separately.
  • ensure_type -> cast_{implicit,assign,explicit} or FunctionCall::new_cast
    • FunctionCall::new_with_return_type should not be abused to bypass infer_type. It will be deprecated in a later PR.

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)

@CLAassistant
Copy link

CLAassistant commented Apr 11, 2022

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #1759 (98614e8) into main (30056b6) will increase coverage by 0.08%.
The diff coverage is 97.47%.

@@            Coverage Diff             @@
##             main    #1759      +/-   ##
==========================================
+ Coverage   71.12%   71.21%   +0.08%     
==========================================
  Files         604      604              
  Lines       78094    78261     +167     
==========================================
+ Hits        55546    55734     +188     
+ Misses      22548    22527      -21     
Flag Coverage Δ
rust 71.21% <97.47%> (+0.08%) ⬆️

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

Impacted Files Coverage Δ
src/common/src/types/mod.rs 63.70% <ø> (+1.01%) ⬆️
src/frontend/src/expr/function_call.rs 95.41% <73.68%> (-3.69%) ⬇️
src/frontend/src/expr/type_inference.rs 96.14% <99.45%> (+2.07%) ⬆️
src/frontend/src/binder/expr/function.rs 91.52% <100.00%> (+0.45%) ⬆️
src/frontend/src/binder/expr/mod.rs 78.62% <100.00%> (-0.51%) ⬇️
src/frontend/src/binder/values.rs 97.10% <100.00%> (+8.49%) ⬆️
src/frontend/src/expr/mod.rs 90.37% <100.00%> (-0.15%) ⬇️
...rc/frontend/src/optimizer/plan_node/logical_agg.rs 97.49% <100.00%> (+<0.01%) ⬆️
src/common/src/types/ordered_float.rs 23.70% <0.00%> (+0.19%) ⬆️
... and 3 more

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

@xiangjinwu xiangjinwu changed the title feat(binder): WIP align type cast-ability with pg feat(binder): align type cast-ability with pg Apr 11, 2022
@xiangjinwu xiangjinwu marked this pull request as ready for review April 11, 2022 12:02
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.

Rest LGTM. Great job!

src/frontend/src/binder/expr/function.rs Show resolved Hide resolved
@xiangjinwu xiangjinwu enabled auto-merge (squash) April 12, 2022 03:53
@xiangjinwu xiangjinwu merged commit cba8e3c into main Apr 12, 2022
@xiangjinwu xiangjinwu deleted the rust-frontend-cast branch April 12, 2022 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants