kangpinghuang commented on a change in pull request #3424:
URL: https://github.com/apache/incubator-doris/pull/3424#discussion_r422450785
##########
File path: be/src/olap/olap_cond.cpp
##########
@@ -637,7 +637,7 @@ bool Conditions::delete_conditions_eval(const RowCursor&
row) const {
}
for (auto& each_cond : _columns) {
- if (each_cond.second->is_key() && !each_cond.second->eval(row)) {
+ if (_cond_column_is_key_or_duplicate(each_cond.second) &&
!each_cond.second->eval(row)) {
Review comment:
I think check agg type is more clear?
##########
File path: be/src/olap/delete_handler.cpp
##########
@@ -105,10 +105,10 @@ OLAPStatus DeleteConditionHandler::check_condition_valid(
// 检查指定的列是不是key，是不是float或doulbe类型
const TabletColumn& column = schema.column(field_index);
- if (!column.is_key()
+ if ((!column.is_key() && schema.keys_type() != KeysType::DUP_KEYS)
Review comment:
just check column.agg_type = AGG_NONE？
##########
File path: be/src/olap/rowset/alpha_rowset.cpp
##########
@@ -300,7 +300,7 @@ OLAPStatus AlphaRowset::init() {
if (segment_group_meta.zone_maps_size() != 0) {
size_t zone_maps_size = segment_group_meta.zone_maps_size();
- size_t num_key_columns = _schema->num_key_columns();
+ size_t num_key_columns = _schema->keys_type() ==
KeysType::DUP_KEYS ? _schema->num_columns() : _schema->num_key_columns();
Review comment:
I think we should change the num_key_columns name.
num_key_columns will be ambiguous.
##########
File path: fe/src/main/java/org/apache/doris/load/DeleteHandler.java
##########
@@ -478,9 +479,11 @@ private void checkDeleteV2(OlapTable table, Partition
partition, List<Predicate>
}
Column column = nameToColumn.get(columnName);
- if (!column.isKey()) {
+ if (!column.isKey() && table.getKeysType() != KeysType.DUP_KEYS
Review comment:
should add some comment to explain why float and double type delete
predicate can not be supported.
##########
File path: be/src/olap/wrapper_field.h
##########
@@ -81,7 +81,7 @@ class WrapperField {
char* ptr() const { return _field_buf + 1; }
size_t size() const { return _rep->size(); }
size_t field_size() const { return _rep->field_size(); }
- bool is_null() const { return *reinterpret_cast<bool*>(_field_buf); }
+ bool is_null() const { return _field_buf == nullptr ||
*reinterpret_cast<bool*>(_field_buf); }
Review comment:
why to add this condition?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
