You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/12/12 09:15:00 UTC

[GitHub] [doris] liaoxin01 commented on a diff in pull request #14995: [fix](merge-on-write) delete all rows with same key in all pre segments

liaoxin01 commented on code in PR #14995:
URL: https://github.com/apache/doris/pull/14995#discussion_r1045565910


##########
be/src/olap/tablet.cpp:
##########
@@ -2044,19 +2048,26 @@ Status Tablet::calc_delete_bitmap(RowsetId rowset_id,
 }
 
 Status Tablet::_check_pk_in_pre_segments(
-        const std::vector<segment_v2::SegmentSharedPtr>& pre_segments, const Slice& key,
-        const Version& version, DeleteBitmapPtr delete_bitmap, RowLocation* loc) {
+        RowsetId rowset_id, const std::vector<segment_v2::SegmentSharedPtr>& pre_segments,
+        const Slice& key, const Version& version, DeleteBitmapPtr delete_bitmap, RowLocation* loc) {
     for (auto it = pre_segments.rbegin(); it != pre_segments.rend(); ++it) {
         auto st = (*it)->lookup_row_key(key, loc);
         CHECK(st.ok() || st.is<NOT_FOUND>() || st.is<ALREADY_EXIST>());
         if (st.is<NOT_FOUND>()) {
             continue;
         } else if (st.ok() && _schema->has_sequence_col() &&
-                   delete_bitmap->contains({loc->rowset_id, loc->segment_id, version.first},
+                   delete_bitmap->contains({rowset_id, loc->segment_id, version.first},
                                            loc->row_id)) {
             // if has sequence col, we continue to compare the sequence_id of
             // all segments, util we find an existing key.
             continue;
+        } else if (st.ok() && !_schema->has_sequence_col()) {
+            // delete all rows with same key in all pre segments
+            if (!delete_bitmap->contains({rowset_id, loc->segment_id, version.first},
+                                         loc->row_id)) {
+                delete_bitmap->add({rowset_id, loc->segment_id, version.first}, loc->row_id);
+            }
+            continue;

Review Comment:
   Don't need continue here? Just need to find the key in the previous segment, instead of traversing all segments for mow without sequence col.



##########
be/src/olap/tablet.cpp:
##########
@@ -1999,8 +1999,8 @@ Status Tablet::calc_delete_bitmap(RowsetId rowset_id,
                 RowLocation loc;
                 // first check if exist in pre segment
                 if (check_pre_segments) {
-                    auto st = _check_pk_in_pre_segments(pre_segments, *key, dummy_version,
-                                                        delete_bitmap, &loc);
+                    auto st = _check_pk_in_pre_segments(rowset_id, pre_segments, *key,
+                                                        dummy_version, delete_bitmap, &loc);
                     if (st.ok()) {
                         delete_bitmap->add({loc.rowset_id, loc.segment_id, dummy_version.first},

Review Comment:
   Don't need this code, because already added in `_check_pk_in_pre_segments`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org