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

refactor: load parquet use own can_cast_to() #16072

Closed
wants to merge 3 commits into from

Conversation

youngsofun
Copy link
Member

@youngsofun youngsofun commented Jul 18, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

This PR updates the dependency from Arrow’s can_cast_types to our own can_cast_to() function.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Jul 18, 2024
// we mainly care about which types can/cannot cast to dest_type.
// the match is written in a way to make it easier to read this info.
// try to use less _ to avoid miss something
match (dest_type, src_type) {
Copy link
Member

Choose a reason for hiding this comment

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

seems put src_type on left is more customary.

Copy link
Member Author

Choose a reason for hiding this comment

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

the comments explain the reason.

// we mainly care about which types can/cannot cast to dest_type.
// the match is written in a way to make it easier to read this info.

Copy link
Member Author

@youngsofun youngsofun Jul 18, 2024

Choose a reason for hiding this comment

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

To be more specific, our use case is as follows: we have a dest type and several src types. We need to determine if the cast between them is valid.

and for each case (row), there is only one dest and multi src, so put dest in front is more alined and readable

src/query/expression/src/type_check.rs Show resolved Hide resolved
@youngsofun youngsofun changed the title refactor: load parquet use own can_auto_cast_to() refactor: load parquet use own can_cast_to() Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-refactor this PR changes the code base without new features or bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants