zhannngchen commented on code in PR #41232:
URL: https://github.com/apache/doris/pull/41232#discussion_r1776511036
##########
be/src/exec/tablet_info.cpp:
##########
@@ -201,6 +205,28 @@ Status OlapTableSchemaParam::init(const
TOlapTableSchemaParam& tschema) {
"different from BE.");
}
_auto_increment_column_unique_id =
tschema.auto_increment_column_unique_id;
+ if (tschema.__isset.partial_update_new_row_policy) {
+ switch (tschema.partial_update_new_row_policy) {
+ case doris::TPartialUpdateNewRowPolicy::APPEND: {
+ _partial_update_new_row_policy =
PartialUpdateNewRowPolicyPB::APPEND;
+ break;
+ }
+ case doris::TPartialUpdateNewRowPolicy::IGNORE: {
+ _partial_update_new_row_policy =
PartialUpdateNewRowPolicyPB::IGNORE;
+ break;
+ }
+ case doris::TPartialUpdateNewRowPolicy::ERROR: {
+ _partial_update_new_row_policy =
PartialUpdateNewRowPolicyPB::ERROR;
+ break;
+ }
+ default: {
+ return Status::InternalError(
Review Comment:
use `InvalidArgument` error
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -513,6 +514,8 @@ public class SessionVariable implements Serializable,
Writable {
public static final String ENABLE_UNIQUE_KEY_PARTIAL_UPDATE =
"enable_unique_key_partial_update";
+ public static final String PARTIAL_UPDATE_NEW_ROW_POLICY =
"partial_update_new_row_policy";
Review Comment:
`partial_update_new_key_policy` is better
##########
be/src/olap/rowset/segment_v2/segment_writer.cpp:
##########
@@ -475,16 +475,15 @@ Status SegmentWriter::probe_key_for_mow(
specified_rowsets, &loc,
_mow_context->max_version,
segment_caches, &rowset);
if (st.is<KEY_NOT_FOUND>()) {
- if (_opts.rowset_ctx->partial_update_info->is_strict_mode) {
- ++stats.num_rows_filtered;
+ auto ignore_cb = [&]() {
Review Comment:
The `ignore` operation will not work after compaction, compaction will
ignore delete bitmap, and keep the latest key, unless you add a delete sign to
such row.
But adding delete sign might have some issue if user specified sequence
column.
##########
regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_new_row_policy.groovy:
##########
@@ -0,0 +1,140 @@
+
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_partial_update_new_row_policy", "p0") {
+
+ def tableName = "test_partial_update_only_keys"
+ sql """ DROP TABLE IF EXISTS ${tableName} force"""
+ sql """ CREATE TABLE ${tableName} (
+ `k` BIGINT NOT NULL,
+ `c1` int,
+ `c2` int,
+ `c3` int)
+ UNIQUE KEY(`k`) DISTRIBUTED BY HASH(`k`) BUCKETS 1
+ PROPERTIES(
+ "replication_num" = "1",
+ "enable_unique_key_merge_on_write" = "true"); """
+ sql """insert into ${tableName} select number,number,number,number from
numbers("number"="3");"""
+ qt_sql """select * from ${tableName} order by k;"""
+
+ def checkVariable = { expected ->
+ def res = sql_return_maparray """show variables where
Variable_name="partial_update_new_row_policy";""";
+ assertTrue(res[0].Value.equalsIgnoreCase(expected));
+ }
+
+ sql "set enable_unique_key_partial_update=true;"
+ sql "sync"
+
+ sql """set partial_update_new_row_policy="APPEND";"""
+ sql "sync;"
+ checkVariable("APPEND")
+ explain {
+ sql "insert into ${tableName}(k,c1) values(0,10),(3,10),(4,10),(5,10);"
+ contains "PARTIAL_UPDATE_NEW_ROW_POLICY: APPEND"
+ }
+ sql "insert into ${tableName}(k,c1) values(0,10),(3,10),(4,10),(5,10);"
+ qt_append """select * from ${tableName} order by k;"""
+
+
+ sql """set partial_update_new_row_policy="ignore";"""
+ sql "sync;"
+ checkVariable("IGNORE")
+ explain {
+ sql "insert into ${tableName}(k,c2) values(1,20),(3,80),(6,80),(7,80);"
+ contains "PARTIAL_UPDATE_NEW_ROW_POLICY: IGNORE"
+ }
+ sql "insert into ${tableName}(k,c2) values(1,20),(3,80),(6,80),(7,80);"
+ qt_ignore """select * from ${tableName} order by k;"""
+
+
+ sql """set partial_update_new_row_policy="ERROR";"""
+ sql "sync;"
+ checkVariable("ERROR")
+ explain {
+ sql "insert into ${tableName}(k,c2) values(1,30),(2,30);"
+ contains "PARTIAL_UPDATE_NEW_ROW_POLICY: ERROR"
+ }
+ sql "insert into ${tableName}(k,c2) values(1,30),(2,30);"
+ qt_error1 """select * from ${tableName} order by k;"""
+ test {
+ sql "insert into ${tableName}(k,c2) values(1,30),(10,999),(11,999);"
+ exception "[E-7003]Can't append new rows in partial update when
partial_update_new_row_policy is ERROR"
+ }
+ qt_error2 """select * from ${tableName} order by k;"""
+
+
+ sql """set partial_update_new_row_policy=default;"""
+ sql "sync;"
+ checkVariable("APPEND")
+ test {
+ sql """set partial_update_new_row_policy="invalid";"""
+ exception "partial_update_new_row_policyshould be one of {'APPEND',
'IGNORE', 'ERROR'}, but found invalid"
+ }
+ checkVariable("APPEND")
+
+ sql "set enable_unique_key_partial_update=false;"
+ sql "sync;"
+
+ streamLoad {
+ table "${tableName}"
+ set 'column_separator', ','
+ set 'format', 'csv'
+ set 'columns', 'k,c3'
+ set 'partial_columns', 'true'
+ set 'partial_update_new_row_policy', 'append'
+ file 'row_policy1.csv'
+ time 10000
+ }
+ qt_append """select * from ${tableName} order by k;"""
+
+ streamLoad {
+ table "${tableName}"
+ set 'column_separator', ','
+ set 'format', 'csv'
+ set 'columns', 'k,c3'
+ set 'partial_columns', 'true'
+ set 'partial_update_new_row_policy', 'IGNORE'
+ file 'row_policy2.csv'
+ time 10000
+ }
+ qt_ignore """select * from ${tableName} order by k;"""
+
+ streamLoad {
+ table "${tableName}"
+ set 'column_separator', ','
+ set 'format', 'csv'
+ set 'columns', 'k,c3'
+ set 'partial_columns', 'true'
Review Comment:
also test on partial_columns:false
##########
regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_new_row_policy.groovy:
##########
@@ -0,0 +1,140 @@
+
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_partial_update_new_row_policy", "p0") {
+
+ def tableName = "test_partial_update_only_keys"
+ sql """ DROP TABLE IF EXISTS ${tableName} force"""
+ sql """ CREATE TABLE ${tableName} (
+ `k` BIGINT NOT NULL,
+ `c1` int,
+ `c2` int,
+ `c3` int)
+ UNIQUE KEY(`k`) DISTRIBUTED BY HASH(`k`) BUCKETS 1
Review Comment:
Add tests on different tables, make sure the new session variable and header
won't affect non-mow tables
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
