fix: block type reset after dict filter#63168
Merged
Gabriel39 merged 2 commits intoMay 20, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
|
@yoock Please add regression test cases. |
Contributor
Author
本质上就是RowGroupReader::next_batch结束时必须调用_convert_dict_cols_to_string_cols(block);，但是这里漏了 |
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed the full PR diff and existing review context. I found one blocking issue in BE Parquet lazy-read error handling: the new call ignores a Status-returning conversion helper, which violates Doris error propagation rules and can also fail compilation because Status is [[nodiscard]].
Critical checkpoint conclusions:
- Goal/test: The PR appears intended to keep dictionary-filtered columns converted to string on an early EOF lazy-read path. The change is not safely implemented because conversion failures are not propagated; no new test is included for this edge path.
- Scope/clarity: The change is small and focused, but needs the same RETURN_IF_ERROR handling used by adjacent paths.
- Concurrency/lifecycle/config/compatibility: No new concurrency, lifecycle, config, persistence, storage-format, or FE-BE compatibility concerns found in this one-line change.
- Parallel paths: Neighboring no-row/filter-all paths already propagate _convert_dict_cols_to_string_cols errors; this new path should match them.
- Error handling: Blocking issue found; Status is ignored.
- Test coverage/results: No test or result changes were added. Given the fix is a direct error-propagation correction, existing coverage may be sufficient after updating the call, but this specific lazy EOF path is not newly covered.
- Observability/performance/memory: No additional observability, performance, or memory issues found.
- User focus: No additional user-provided review focus was specified.
| DCHECK_EQ(pre_read_rows + _cached_filtered_rows, 0); | ||
| *read_rows = 0; | ||
| *batch_eof = true; | ||
| _convert_dict_cols_to_string_cols(block); |
Contributor
There was a problem hiding this comment.
This helper returns Status and Status is [[nodiscard]], so this call must be propagated. More importantly, if _get_block_column_pos() or convert_dict_column_to_string_column() fails, this path currently returns OK with the block left in the dictionary-code representation, unlike all adjacent _convert_dict_cols_to_string_cols call sites that use RETURN_IF_ERROR. Please keep the same error handling here.
Suggested change
| _convert_dict_cols_to_string_cols(block); | |
| RETURN_IF_ERROR(_convert_dict_cols_to_string_cols(block)); |
Contributor
|
/review |
Contributor
|
/run buildall |
Contributor
|
run buildall |
Contributor
TPC-H: Total hot run time: 30897 ms |
Contributor
TPC-DS: Total hot run time: 169949 ms |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
|
/review |
Gabriel39
approved these changes
May 20, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
May 20, 2026
### What problem does this PR solve? Fix parquet reader reporting: Read parquet file xxxxx.parquet failed, reason = [INTERNAL_ERROR]comparison must input two same type column or column type is decimalv3/numeric, lhs=Int32, rhs=String --------- Co-authored-by: wanglong16 <wanglong16@xiaomi.com>
github-actions Bot
pushed a commit
that referenced
this pull request
May 20, 2026
### What problem does this PR solve? Fix parquet reader reporting: Read parquet file xxxxx.parquet failed, reason = [INTERNAL_ERROR]comparison must input two same type column or column type is decimalv3/numeric, lhs=Int32, rhs=String --------- Co-authored-by: wanglong16 <wanglong16@xiaomi.com>
yiguolei
pushed a commit
that referenced
this pull request
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
修复读取parquet时，当上一个row group有dict filter时未将block的类型从Int32重置为String，导致报如下错误：
Read parquet file xxxxx.parquet failed, reason = [INTERNAL_ERROR]comparison must input two same type column or column type is decimalv3/numeric, lhs=Int32, rhs=String
在有些情况下会触发如下代码导致be core：