fix: core dump in macos running sql_cluster_test #1892
Conversation
if case contains resource field
`DistributeWindowIterator::SeekToFirst` will set `it_` even there is no data locally, this lead to use `it_` instead `kv_it_` to fetch row data when using `GetKey()`, which will core the test
github-actions
bot
added
execute-engine
aceforeverd
requested review from
dl239,
zhanghaohit,
tobegit3hub and
vagetablechicken
and removed request for
dl239
Codecov Report
@@ Coverage Diff @@
## main #1892 +/- ##
============================================
- Coverage 75.69% 75.69% -0.01%
Complexity 347 347
============================================
Files 613 613
Lines 117018 117031 +13
Branches 1024 1024
============================================
+ Hits 88579 88584 +5
- Misses 28230 28238 +8
Partials 209 209
Continue to review full report at Codecov.
|
aceforeverd
changed the title
| kv_it_ = kv.second->Traverse(tid_, cur_pid_, index_name_, "", 0, FLAGS_traverse_cnt_limit, count); | ||
| if (kv_it_ && kv_it_->Valid()) { | ||
| response_vec_.emplace_back(kv_it_->GetResponse()); | ||
| auto it = kv.second->Traverse(tid_, kv.first, index_name_, "", 0, FLAGS_traverse_cnt_limit, count); |
There was a problem hiding this comment.
the Seek has the same problem. it's better to check it_ validation in GetKey and GetValue function
There was a problem hiding this comment.
we should ensure only zero or one of it_ and kv_it_ is not null.
So the check here is whether it_ and kv_it_ both not null.
It is it_ and kv_it_ 's duty to check valid itself before GetKey or GetValue
There was a problem hiding this comment.
we should improve type safety for iterators, in both definition and implementation. E.g the GetKey method of WindowIterator should mark as const
sql_cluster_testcore in darwin: when there is no partition data for a table locally, theDistributeWindowIterator::SeekToFirstsetit_still. This cores the program when user want toDistributeWindowIterator::GetKey()becauseit_is not null, so it try to get key data fromit_toydb_run_enginebootstrapping