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](BE) Uniform Expr, Block, Columns size-relative interface (Part II) #43218

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

zclllyybb
Copy link
Contributor

@zclllyybb zclllyybb commented Nov 4, 2024

part 2 of #42930

two modifications:

  1. when parsing data of json array, cast to target type when parse int data no matter what source type is.
  2. in explode_numbers, the argument column type is certain Int32. so reduce some virtual function call.

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

be/src/util/simd/bits.h Outdated Show resolved Hide resolved
be/src/vec/exprs/vcompound_pred.h Outdated Show resolved Hide resolved
be/src/vec/exprs/vectorized_agg_fn.h Outdated Show resolved Hide resolved
be/src/vec/exprs/vectorized_fn_call.h Outdated Show resolved Hide resolved
be/src/vec/functions/array/function_array_index.h Outdated Show resolved Hide resolved
be/src/vec/functions/function.h Outdated Show resolved Hide resolved
BiteTheDDDDt pushed a commit that referenced this pull request Nov 8, 2024
…Part I) (#42930)

## Proposed changes
In this pr we refactored some base interface to avoid narrow cast with
data consistent kept.

We uniformed these type of numbers:
```
columns in Block        - U32 (just use no more than U16)
Rows in Block           - U64 (others U32, ColumnString64 U64)
column_id in expr arg   - I32 (because -1 is meaningful)
child of Expr           - U16 (FE use int32, have a restriction of 10000. adjustable but no guarantee for result)
```

So, for functions' argument column id, we use `uint32` to replace
`size_t`.
For rows, we use `size_t` to replace some `int`.
For child of expr, we cast it as `uint_16` to use, rather than `size_t`
as `vector::size()`

Refactor of table functions: #43218
@zclllyybb
Copy link
Contributor Author

run buildall

Copy link
Contributor

github-actions bot commented Nov 8, 2024

PR approved by anyone and no changes requested.

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.90% (9864/26027)
Line Coverage: 29.10% (82273/282747)
Region Coverage: 28.24% (42336/149915)
Branch Coverage: 24.79% (21438/86474)
Coverage Report: http://coverage.selectdb-in.cc/coverage/0c9e40fe19b6c36ca616964387fa8e479b9f0387_0c9e40fe19b6c36ca616964387fa8e479b9f0387/report/index.html

Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 8, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

PR approved by at least one committer and no changes requested.

@zhangstar333 zhangstar333 merged commit 279c162 into apache:master Nov 11, 2024
27 of 29 checks passed
924060929 added a commit that referenced this pull request Nov 18, 2024
924060929 added a commit to 924060929/incubator-doris that referenced this pull request Nov 18, 2024
fix macos compile failed, introduced by apache#40813, apache#42930, apache#43218, apache#43289

(cherry picked from commit ded2190)
924060929 added a commit that referenced this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants