[test](geo) drop redundant assert_cast on already-typed null_map column#63701
Closed
airborne12 wants to merge 1 commit into
Closed
[test](geo) drop redundant assert_cast on already-typed null_map column#63701airborne12 wants to merge 1 commit into
airborne12 wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
3 tasks
Contributor
TPC-H: Total hot run time: 31574 ms |
Contributor
TPC-DS: Total hot run time: 172164 ms |
Member
Author
|
BeUt 953876 finished SUCCESS — the missing change (commit 3da727fc366) restores BE-UT compilation on master after the interaction between #63491 (strongly typed null_map accessor) and #63049 (new geo test). Without this fix, every PR running BE-UT fails the same way (see #63692 953796/953904). Please consider expediting. |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: close #N/A Problem Summary: BE-UT has been failing master-wide since both PR apache#63491 (which strongly typed `ColumnNullable::get_null_map_column[_ptr]()` to `ColumnUInt8`) and PR apache#63049 (which added `functions_geo_test.cpp`) landed today. The new test calls ```cpp assert_cast<ColumnUInt8*>(nullable_input->get_null_map_column_ptr().get()) ->insert_value(0); ``` but the inner expression is already `ColumnUInt8*`, so the cast triggers the same-type static_assert added by PR apache#63133 to `src/core/assert_cast.h`: ``` static assertion failed due to requirement '!std::is_same_v<doris::ColumnVector<doris::TYPE_BOOLEAN> *, doris::ColumnVector<doris::TYPE_BOOLEAN> *>': assert_cast is redundant for the same type after removing cv/ref qualifiers ``` That kills `doris_be_test` compilation on every PR that runs BE-UT. Use the strongly typed `get_null_map_column()` (which returns `ColumnUInt8&`) directly so the cast is no longer needed. ### Release note None (test-only change, restores BE-UT compilation on master). ### Check List (For Author) - Test: - Compile-check on local ASAN tree: the affected translation unit now builds clean (`ninja test/CMakeFiles/doris_be_test.dir/exprs/function/geo/functions_geo_test.cpp.o`). - Behavior changed: No - Does this need documentation: No
airborne12
force-pushed
the
fix-geo-test-assert-cast
branch
from
3da727f to
52c346d
Compare
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
2 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.
Proposed changes
Issue Number: close #N/A
What problem does this PR solve?
BE-UT has been failing master-wide since both #63491 (strongly typed
ColumnNullable::get_null_map_column[_ptr]()to `ColumnUInt8`) and #63049 (added `functions_geo_test.cpp`) landed today. The new test calls:```cpp
assert_cast<ColumnUInt8*>(nullable_input->get_null_map_column_ptr().get())
->insert_value(0);
```
The inner expression is now already `ColumnUInt8*`, so the cast hits the same-type static_assert added by #63133 in `src/core/assert_cast.h`:
```
static assertion failed due to requirement
'!std::is_same_v<doris::ColumnVectordoris::TYPE_BOOLEAN *,
doris::ColumnVectordoris::TYPE_BOOLEAN *>':
assert_cast is redundant for the same type after removing cv/ref qualifiers
```
That kills `doris_be_test` compilation on every PR that exercises BE-UT (e.g. #63692 953796, ##63637 953796).
Use the strongly typed `get_null_map_column()` (which returns `ColumnUInt8&`) directly so no cast is needed.
Release note
None (test-only change, restores BE-UT compilation on master).
Check List (For Author)