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 2020/06/24 13:08:34 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
caiconghui opened a new pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman commented on a change in pull request #3943: [Bug] Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#discussion_r446589503
##########
File path: be/src/olap/rowset/segment_v2/segment_iterator.cpp
##########
@@ -253,30 +253,37 @@ Status SegmentIterator::_get_row_ranges_from_conditions(RowRanges* condition_row
_opts.stats->rows_bf_filtered += (pre_size - condition_row_ranges->count());
RowRanges zone_map_row_ranges = RowRanges::create_single(num_rows());
- std::map<int, std::vector<CondColumn*>> column_delete_conditions;
- // zone map will use delete conditions
- for (auto& delete_condition : _opts.delete_conditions) {
- for (auto& column_condition : delete_condition->columns()) {
- cids.insert(column_condition.first);
- std::vector<CondColumn*>& conditions = column_delete_conditions[column_condition.first];
- conditions.emplace_back(column_condition.second);
- }
- }
+ // second filter data by zone map
for (auto& cid : cids) {
// get row ranges by zone map of this column,
RowRanges column_row_ranges = RowRanges::create_single(num_rows());
- CondColumn* column_cond = nullptr;
- if (_opts.conditions != nullptr) {
- column_cond = _opts.conditions->get_column(cid);
- }
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
Review comment:
`_opts.conditions` may be `nullptr` here.
You need to check it before use, like the old way.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman edited a comment on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman edited a comment on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649570385
I think the root cause is we lost the relationship of the 2 delete conditions in same delete stmt.
For example, the following 2 cases should has different result:
Base data:
```
k1, k2
1, 2
1, 3
```
case 1:
`delete from tbl where k1=1 and k2=2`;
case 2:
`delete from tbl where k1=1;`
`delete from tbl where k2=2;`
We treat the above 2 cases as same, which is incorrect.
So I think only change the `intersect` to `union` is not the right way to fix the bug.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman commented on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649591633
I am not familiar with this part. But following is the pseudocode I thought:
```
All DeleteConditions:
DeleteConditions 1: k1 = 1 and k2 = 2;
DeleteConditions 2: k1 = 4
DeleteConditions 3: k1 = 5
RowRanges result;
foreach(DeleteConditions dcs : All DeleteConditions) {
RowRanges rr;
foreach(DeleteCondition dc : dcs) {
rr = rr UNION (rows filtered by dc)
}
result = result INTERSECT rr;
}
return result;
```
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman commented on a change in pull request #3943: [Bug] Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#discussion_r446474144
##########
File path: be/src/olap/rowset/segment_v2/segment_iterator.cpp
##########
@@ -253,30 +253,37 @@ Status SegmentIterator::_get_row_ranges_from_conditions(RowRanges* condition_row
_opts.stats->rows_bf_filtered += (pre_size - condition_row_ranges->count());
RowRanges zone_map_row_ranges = RowRanges::create_single(num_rows());
- std::map<int, std::vector<CondColumn*>> column_delete_conditions;
- // zone map will use delete conditions
- for (auto& delete_condition : _opts.delete_conditions) {
- for (auto& column_condition : delete_condition->columns()) {
- cids.insert(column_condition.first);
- std::vector<CondColumn*>& conditions = column_delete_conditions[column_condition.first];
- conditions.emplace_back(column_condition.second);
- }
- }
+ // second filter data by zone map
for (auto& cid : cids) {
// get row ranges by zone map of this column,
RowRanges column_row_ranges = RowRanges::create_single(num_rows());
- CondColumn* column_cond = nullptr;
- if (_opts.conditions != nullptr) {
- column_cond = _opts.conditions->get_column(cid);
- }
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
RETURN_IF_ERROR(
- _column_iterators[cid]->get_row_ranges_by_zone_map(
- column_cond,
- column_delete_conditions[cid],
- &column_row_ranges));
- // intersection different columns's row ranges to get final row ranges by zone map
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
+ nullptr,
+ &column_row_ranges));
+ // intersect different columns's row ranges to get final row ranges by zone map
RowRanges::ranges_intersection(zone_map_row_ranges, column_row_ranges, &zone_map_row_ranges);
}
+
+ // final filter data with delete conditions
+ for (auto& delete_condition : _opts.delete_conditions) {
+ RowRanges delete_condition_row_ranges = RowRanges::create_single(0);
+ for (auto& delete_column_condition : delete_condition->columns()) {
+ const int32_t cid = delete_column_condition.first;
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
+ RowRanges single_delete_condition_row_ranges = RowRanges::create_single(num_rows());
+ RETURN_IF_ERROR(
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
Review comment:
Should the `column_cond` be null here?
We already filter the data by column conditions before.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman removed a comment on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman removed a comment on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649555591
It is strange that the case you described in issue truly has problem. But the following case is OK:
```
mysql> create table tbl4 (k1 int, k2 int) distributed by hash(k1) buckets 1 properties("replication_num" = "1");
Query OK, 0 rows affected (0.01 sec)
mysql> insert into tbl4 values(1,2),(3,4);
Query OK, 2 rows affected (0.04 sec)
{'label':'insert_20ddaec3-5d29-43a9-b065-7d3d680e9d33', 'status':'VISIBLE', 'txnId':'3014'}
mysql> select * from tbl4;
+------+------+
| k1 | k2 |
+------+------+
| 1 | 2 |
| 3 | 4 |
+------+------+
2 rows in set (0.01 sec)
mysql> delete from tbl4 where k2=2 and k1=1;
Query OK, 0 rows affected (0.04 sec)
{'label':'delete_1e19b1ba-0a42-48ee-989c-813758ea9a36', 'status':'VISIBLE', 'txnId':'3015'}
mysql> select * from tbl4;
+------+------+
| k1 | k2 |
+------+------+
| 3 | 4 |
+------+------+
1 row in set (0.01 sec)
```
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman edited a comment on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman edited a comment on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649555591
It is strange that the case you described in issue truly has problem. But the following case is OK:
```
mysql> create table tbl4 (k1 int, k2 int) distributed by hash(k1) buckets 1 properties("replication_num" = "1");
Query OK, 0 rows affected (0.01 sec)
mysql> insert into tbl4 values(1,2),(3,4);
Query OK, 2 rows affected (0.04 sec)
{'label':'insert_20ddaec3-5d29-43a9-b065-7d3d680e9d33', 'status':'VISIBLE', 'txnId':'3014'}
mysql> select * from tbl4;
+------+------+
| k1 | k2 |
+------+------+
| 1 | 2 |
| 3 | 4 |
+------+------+
2 rows in set (0.01 sec)
mysql> delete from tbl4 where k2=2 and k1=1;
Query OK, 0 rows affected (0.04 sec)
{'label':'delete_1e19b1ba-0a42-48ee-989c-813758ea9a36', 'status':'VISIBLE', 'txnId':'3015'}
mysql> select * from tbl4;
+------+------+
| k1 | k2 |
+------+------+
| 3 | 4 |
+------+------+
1 row in set (0.01 sec)
```
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman commented on a change in pull request #3943: [Bug] Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#discussion_r446588788
##########
File path: be/src/olap/rowset/segment_v2/segment_iterator.cpp
##########
@@ -253,30 +253,37 @@ Status SegmentIterator::_get_row_ranges_from_conditions(RowRanges* condition_row
_opts.stats->rows_bf_filtered += (pre_size - condition_row_ranges->count());
RowRanges zone_map_row_ranges = RowRanges::create_single(num_rows());
- std::map<int, std::vector<CondColumn*>> column_delete_conditions;
- // zone map will use delete conditions
- for (auto& delete_condition : _opts.delete_conditions) {
- for (auto& column_condition : delete_condition->columns()) {
- cids.insert(column_condition.first);
- std::vector<CondColumn*>& conditions = column_delete_conditions[column_condition.first];
- conditions.emplace_back(column_condition.second);
- }
- }
+ // second filter data by zone map
for (auto& cid : cids) {
// get row ranges by zone map of this column,
RowRanges column_row_ranges = RowRanges::create_single(num_rows());
- CondColumn* column_cond = nullptr;
- if (_opts.conditions != nullptr) {
- column_cond = _opts.conditions->get_column(cid);
- }
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
RETURN_IF_ERROR(
- _column_iterators[cid]->get_row_ranges_by_zone_map(
- column_cond,
- column_delete_conditions[cid],
- &column_row_ranges));
- // intersection different columns's row ranges to get final row ranges by zone map
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
+ nullptr,
+ &column_row_ranges));
+ // intersect different columns's row ranges to get final row ranges by zone map
RowRanges::ranges_intersection(zone_map_row_ranges, column_row_ranges, &zone_map_row_ranges);
}
+
+ // final filter data with delete conditions
+ for (auto& delete_condition : _opts.delete_conditions) {
+ RowRanges delete_condition_row_ranges = RowRanges::create_single(0);
+ for (auto& delete_column_condition : delete_condition->columns()) {
+ const int32_t cid = delete_column_condition.first;
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
+ RowRanges single_delete_condition_row_ranges = RowRanges::create_single(num_rows());
+ RETURN_IF_ERROR(
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
Review comment:
OK
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3943: [Bug] Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#discussion_r446590558
##########
File path: be/src/olap/rowset/segment_v2/segment_iterator.cpp
##########
@@ -253,30 +253,37 @@ Status SegmentIterator::_get_row_ranges_from_conditions(RowRanges* condition_row
_opts.stats->rows_bf_filtered += (pre_size - condition_row_ranges->count());
RowRanges zone_map_row_ranges = RowRanges::create_single(num_rows());
- std::map<int, std::vector<CondColumn*>> column_delete_conditions;
- // zone map will use delete conditions
- for (auto& delete_condition : _opts.delete_conditions) {
- for (auto& column_condition : delete_condition->columns()) {
- cids.insert(column_condition.first);
- std::vector<CondColumn*>& conditions = column_delete_conditions[column_condition.first];
- conditions.emplace_back(column_condition.second);
- }
- }
+ // second filter data by zone map
for (auto& cid : cids) {
// get row ranges by zone map of this column,
RowRanges column_row_ranges = RowRanges::create_single(num_rows());
- CondColumn* column_cond = nullptr;
- if (_opts.conditions != nullptr) {
- column_cond = _opts.conditions->get_column(cid);
- }
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
Review comment:
done
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman commented on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649555591
It is strange that the case you described in issue is truly has problem. But the following case is OK:
```
mysql> create table tbl4 (k1 int, k2 int) distributed by hash(k1) buckets 1 properties("replication_num" = "1");
Query OK, 0 rows affected (0.01 sec)
mysql> insert into tbl4 values(1,2),(3,4);
Query OK, 2 rows affected (0.04 sec)
{'label':'insert_20ddaec3-5d29-43a9-b065-7d3d680e9d33', 'status':'VISIBLE', 'txnId':'3014'}
mysql> select * from tbl4;
+------+------+
| k1 | k2 |
+------+------+
| 1 | 2 |
| 3 | 4 |
+------+------+
2 rows in set (0.01 sec)
mysql> delete from tbl4 where k2=2 and k1=1;
Query OK, 0 rows affected (0.04 sec)
{'label':'delete_1e19b1ba-0a42-48ee-989c-813758ea9a36', 'status':'VISIBLE', 'txnId':'3015'}
mysql> select * from tbl4;
+------+------+
| k1 | k2 |
+------+------+
| 3 | 4 |
+------+------+
1 row in set (0.01 sec)
```
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] caiconghui edited a comment on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
caiconghui edited a comment on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649634223
@morningman it is a good implementation to not select block which not need read, although only change the intersect to union can fix the bug but not an efficient fix.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3943: [Bug] Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#discussion_r446526116
##########
File path: be/src/olap/rowset/segment_v2/segment_iterator.cpp
##########
@@ -253,30 +253,37 @@ Status SegmentIterator::_get_row_ranges_from_conditions(RowRanges* condition_row
_opts.stats->rows_bf_filtered += (pre_size - condition_row_ranges->count());
RowRanges zone_map_row_ranges = RowRanges::create_single(num_rows());
- std::map<int, std::vector<CondColumn*>> column_delete_conditions;
- // zone map will use delete conditions
- for (auto& delete_condition : _opts.delete_conditions) {
- for (auto& column_condition : delete_condition->columns()) {
- cids.insert(column_condition.first);
- std::vector<CondColumn*>& conditions = column_delete_conditions[column_condition.first];
- conditions.emplace_back(column_condition.second);
- }
- }
+ // second filter data by zone map
for (auto& cid : cids) {
// get row ranges by zone map of this column,
RowRanges column_row_ranges = RowRanges::create_single(num_rows());
- CondColumn* column_cond = nullptr;
- if (_opts.conditions != nullptr) {
- column_cond = _opts.conditions->get_column(cid);
- }
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
RETURN_IF_ERROR(
- _column_iterators[cid]->get_row_ranges_by_zone_map(
- column_cond,
- column_delete_conditions[cid],
- &column_row_ranges));
- // intersection different columns's row ranges to get final row ranges by zone map
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
+ nullptr,
+ &column_row_ranges));
+ // intersect different columns's row ranges to get final row ranges by zone map
RowRanges::ranges_intersection(zone_map_row_ranges, column_row_ranges, &zone_map_row_ranges);
}
+
+ // final filter data with delete conditions
+ for (auto& delete_condition : _opts.delete_conditions) {
+ RowRanges delete_condition_row_ranges = RowRanges::create_single(0);
+ for (auto& delete_column_condition : delete_condition->columns()) {
+ const int32_t cid = delete_column_condition.first;
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
+ RowRanges single_delete_condition_row_ranges = RowRanges::create_single(num_rows());
+ RETURN_IF_ERROR(
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
Review comment:
@morningman here we use column_cond to skip zone that not need read, so we don't need to process skip zone with delete conditions
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] morningman commented on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649570385
I think the root cause is we lost the relationship of the 2 delete condition in same delete stmt.
For example, the following 2 cases should has different result:
Base data:
```
k1, k2
1, 2
1, 3
```
case 1:
`delete from tbl where k1=1 and k2=2`;
case 2:
`delete from tbl where k1=1;`
`delete from tbl where k2=2;`
We treat the above 2 cases as same, which is incorrect.
So I think only change the `intersect` to `union` is not the right way to fix the bug.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] chaoyli commented on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649163851
You can describe the solution in the commit message.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] caiconghui commented on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
caiconghui commented on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649634223
@morningman it is a good implementation to filter block which not need read, although only change the intersect to union can fix the bug but not an efficient fix.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #3943: [Bug] Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#discussion_r446526116
##########
File path: be/src/olap/rowset/segment_v2/segment_iterator.cpp
##########
@@ -253,30 +253,37 @@ Status SegmentIterator::_get_row_ranges_from_conditions(RowRanges* condition_row
_opts.stats->rows_bf_filtered += (pre_size - condition_row_ranges->count());
RowRanges zone_map_row_ranges = RowRanges::create_single(num_rows());
- std::map<int, std::vector<CondColumn*>> column_delete_conditions;
- // zone map will use delete conditions
- for (auto& delete_condition : _opts.delete_conditions) {
- for (auto& column_condition : delete_condition->columns()) {
- cids.insert(column_condition.first);
- std::vector<CondColumn*>& conditions = column_delete_conditions[column_condition.first];
- conditions.emplace_back(column_condition.second);
- }
- }
+ // second filter data by zone map
for (auto& cid : cids) {
// get row ranges by zone map of this column,
RowRanges column_row_ranges = RowRanges::create_single(num_rows());
- CondColumn* column_cond = nullptr;
- if (_opts.conditions != nullptr) {
- column_cond = _opts.conditions->get_column(cid);
- }
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
RETURN_IF_ERROR(
- _column_iterators[cid]->get_row_ranges_by_zone_map(
- column_cond,
- column_delete_conditions[cid],
- &column_row_ranges));
- // intersection different columns's row ranges to get final row ranges by zone map
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
+ nullptr,
+ &column_row_ranges));
+ // intersect different columns's row ranges to get final row ranges by zone map
RowRanges::ranges_intersection(zone_map_row_ranges, column_row_ranges, &zone_map_row_ranges);
}
+
+ // final filter data with delete conditions
+ for (auto& delete_condition : _opts.delete_conditions) {
+ RowRanges delete_condition_row_ranges = RowRanges::create_single(0);
+ for (auto& delete_column_condition : delete_condition->columns()) {
+ const int32_t cid = delete_column_condition.first;
+ CondColumn* column_cond = _opts.conditions->get_column(cid);
+ RowRanges single_delete_condition_row_ranges = RowRanges::create_single(num_rows());
+ RETURN_IF_ERROR(
+ _column_iterators[cid]->get_row_ranges_by_zone_map(
+ column_cond,
Review comment:
@morningman here we use column_cond to skip zone that not need read, so we don't need to process skipped zone with delete conditions
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] chaoyli merged pull request #3943: [Bug] Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
chaoyli merged pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] caiconghui removed a comment on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
caiconghui removed a comment on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649634223
@morningman it is a good implementation to not select block which not need read, although only change the intersect to union can fix the bug but not an efficient fix.
----------------------------------------------------------------
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.
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
[GitHub] [incubator-doris] caiconghui commented on pull request #3943: Fix bug that delete stmt with filter condition delete all data from table on segment v2
Posted by GitBox <gi...@apache.org>.
caiconghui commented on pull request #3943:
URL: https://github.com/apache/incubator-doris/pull/3943#issuecomment-649170488
> You can describe the solution in the commit message.
done
----------------------------------------------------------------
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.
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