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): overloaded operator/function resolution with implicit arg cast #3393

Merged
merged 15 commits into from
Jun 27, 2022

Conversation

xiangjinwu
Copy link
Contributor

@xiangjinwu xiangjinwu commented Jun 22, 2022

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

Implements the PostgreSQL logic of resolving overloaded operator/function:
https://www.postgresql.org/docs/current/typeconv-oper.html
https://www.postgresql.org/docs/current/typeconv-func.html

This is necessary to determine the type of literal null used in a function (#3149). And allows us to unify adhoc casting logic scattered in bind_xxx / rewrite_xxx / FunctionCall::new.

The detailed logic of each rule is explained in doc comments. Some prior knowledge about our current type systems (subset of PostgreSQL) is listed here:

We have 13 types from 5 categories. Implicit cast is allowed in the direction of arrow within each category, and the last bolded type is the preferred type of that category.

  • bool
  • smallint/int2/int16 -> int/int4/int32 -> bigint/int8/int64 -> numeric/decimal -> real/float4/float32 -> double precision/float8/float64
  • varchar
  • date -> timestamp -> timestamp with time zone/timestamptz/timestampz
  • time -> interval

Limitations (simplification from PostgreSQL rules):

  • We do not have domain types.
  • We do not have custom operators/functions from multiple schemas (namespaces).
  • We do not support special cast syntax select int4(null); Note that select int(null); is invalid in PostgreSQL as well.
  • Variadic functions, default values, named parameters are not handled here.
  • We do not differentiate operators from functions.
    • We do not have a cache to speedup repeated operator lookup. (PostgreSQL only caches operators but not functions.)

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Refer to a related PR or issue link (optional)

@codecov
Copy link

codecov bot commented Jun 22, 2022

Codecov Report

Merging #3393 (f1e4f31) into main (d362d96) will increase coverage by 0.06%.
The diff coverage is 98.13%.

@@            Coverage Diff             @@
##             main    #3393      +/-   ##
==========================================
+ Coverage   74.31%   74.37%   +0.06%     
==========================================
  Files         769      769              
  Lines      106773   107047     +274     
==========================================
+ Hits        79350    79620     +270     
- Misses      27423    27427       +4     
Flag Coverage Δ
rust 74.37% <98.13%> (+0.06%) ⬆️

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

Impacted Files Coverage Δ
src/frontend/src/expr/mod.rs 87.09% <ø> (ø)
src/frontend/src/expr/type_inference/mod.rs 92.30% <ø> (ø)
src/tests/sqlsmith/src/expr.rs 89.47% <ø> (ø)
src/frontend/src/expr/type_inference/func.rs 98.93% <98.12%> (-0.79%) ⬇️
src/frontend/src/binder/expr/function.rs 81.75% <100.00%> (-0.18%) ⬇️
src/meta/src/hummock/mock_hummock_meta_client.rs 40.56% <0.00%> (-0.95%) ⬇️
src/frontend/src/expr/utils.rs 98.99% <0.00%> (-0.26%) ⬇️
src/storage/src/hummock/local_version_manager.rs 84.37% <0.00%> (-0.16%) ⬇️
src/meta/src/manager/id.rs 96.06% <0.00%> (ø)
... and 1 more

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

@xiangjinwu xiangjinwu force-pushed the rust-implicit-cast-1-actual branch 2 times, most recently from 2e6d9db to 24f28bb Compare June 23, 2022 04:22
@xiangjinwu xiangjinwu marked this pull request as ready for review June 23, 2022 11:44
@xiangjinwu xiangjinwu changed the title feat(binder): (WIP) overloaded operator/function resolution with implicit arg cast feat(binder): overloaded operator/function resolution with implicit arg cast Jun 23, 2022
@xiangjinwu xiangjinwu added the mergify/can-merge Indicates that the PR can be added to the merge queue label Jun 27, 2022
@mergify mergify bot merged commit 7eca500 into main Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergify/can-merge Indicates that the PR can be added to the merge queue type/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants