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

Implement Implicit Casting of Nested Types and Type Combination with MaxLogicalType #3234

Merged
merged 20 commits into from
Apr 16, 2024

Conversation

mxwli
Copy link
Contributor

@mxwli mxwli commented Apr 8, 2024

Nested Implicit Casting Rules:
If the source type and destination type have different LogicalTypeIDs, no implicit cast is possible
If both types are Lists, an implicit cast is possible if their children have an implicit cast from src to dest. Arrays follow the same rule, but an additional check is made to make sure the arrays are of the same length
If both types are Structs, check if the field names match and then check if the matching children have an implicit cast from src to dest.
If both types are Maps, check that the key and value types have implicit casts from src to dest
Union casting seems to be completely unsupported right now, so no implicit casting is allowed.

Update: ARRAY -> LIST implicit casting has been implemented. It goes just the way you would expect it to go.

Max Logical Type Combination Rules:
Case 1:
If at least one type is nonnested,

  1. if one type is ANY/RDF_VARIANT/STRING, cast to the other
  2. if one can cast to the other, take the one with the lowest cost
  3. if both are integral and one is signed & the other is unsigned, combine to lowest width signed integral type
  4. otherwise cast to the value with a larger internalTypeOrder, as defined in types.cpp
    Case 2:
    If both types are nested, the rules are identical to implicit casting rules, but instead of checking "if the src children can implicit cast to the dst children", we check "if the src children can combine with the dst children".

reconfigure

fix tests

new maxlogicaltype

fixes

rdf comparison fix

fix tests

remove added empty lines

add implicit casting between nested types

fix

rename tryCombineVarListTypes to tryCombineListTypes

disallow union casting

Run clang-format

remove test logs

Run clang-format

remove test logs
Copy link
Contributor

@andyfengHKU andyfengHKU left a comment

Choose a reason for hiding this comment

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

Overall LGTM but I need to take another look

src/common/types/types.cpp Outdated Show resolved Hide resolved
src/common/types/types.cpp Show resolved Hide resolved
src/common/types/types.cpp Outdated Show resolved Hide resolved
src/common/types/types.cpp Outdated Show resolved Hide resolved
src/common/types/types.cpp Show resolved Hide resolved
src/binder/expression_binder.cpp Show resolved Hide resolved
src/include/function/cast/vector_cast_functions.h Outdated Show resolved Hide resolved
src/include/common/types/types.h Outdated Show resolved Hide resolved
src/function/vector_cast_functions.cpp Show resolved Hide resolved
@mxwli mxwli merged commit c65b494 into master Apr 16, 2024
17 checks passed
@mxwli mxwli deleted the type-combination branch April 16, 2024 16:50
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.

2 participants