This is an automated email from the ASF dual-hosted git repository.

kxiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 53d255f482 [fix](partial update) remove CHECK on illegal number of 
partial columns (#22319)
53d255f482 is described below

commit 53d255f4822a6f66cd2358bfa0f4f99f8c2adf1d
Author: zhannngchen <48427519+zhannngc...@users.noreply.github.com>
AuthorDate: Fri Jul 28 23:11:58 2023 +0800

    [fix](partial update) remove CHECK on illegal number of partial columns 
(#22319)
---
 be/src/olap/rowset/segment_v2/segment_writer.cpp   | 15 ++--
 .../schema_change/load_without_delete_column.csv   |  2 +-
 .../test_partial_update_schema_change.out          |  7 +-
 .../test_partial_update_schema_change.groovy       | 97 +++++++++++-----------
 4 files changed, 63 insertions(+), 58 deletions(-)

diff --git a/be/src/olap/rowset/segment_v2/segment_writer.cpp 
b/be/src/olap/rowset/segment_v2/segment_writer.cpp
index edc8c43a10..4f303efe01 100644
--- a/be/src/olap/rowset/segment_v2/segment_writer.cpp
+++ b/be/src/olap/rowset/segment_v2/segment_writer.cpp
@@ -320,12 +320,15 @@ void 
SegmentWriter::_serialize_block_to_row_column(vectorized::Block& block) {
 // 3. set columns to data convertor and then write all columns
 Status SegmentWriter::append_block_with_partial_content(const 
vectorized::Block* block,
                                                         size_t row_pos, size_t 
num_rows) {
-    CHECK(block->columns() > _tablet_schema->num_key_columns() &&
-          block->columns() < _tablet_schema->num_columns())
-            << "block columns: " << block->columns()
-            << ", num key columns: " << _tablet_schema->num_key_columns()
-            << ", total schema columns: " << _tablet_schema->num_columns();
-    CHECK(_tablet_schema->keys_type() == UNIQUE_KEYS && 
_opts.enable_unique_key_merge_on_write);
+    if (block->columns() <= _tablet_schema->num_key_columns() ||
+        block->columns() >= _tablet_schema->num_columns()) {
+        return Status::InternalError(
+                fmt::format("illegal partial update block columns: {}, num key 
columns: {}, total "
+                            "schema columns: {}",
+                            block->columns(), 
_tablet_schema->num_key_columns(),
+                            _tablet_schema->num_columns()));
+    }
+    DCHECK(_tablet_schema->keys_type() == UNIQUE_KEYS && 
_opts.enable_unique_key_merge_on_write);
 
     // find missing column cids
     std::vector<uint32_t> missing_cids = _tablet_schema->get_missing_cids();
diff --git 
a/regression-test/data/unique_with_mow_p0/partial_update/schema_change/load_without_delete_column.csv
 
b/regression-test/data/unique_with_mow_p0/partial_update/schema_change/load_without_delete_column.csv
index a0f89729d2..a0dfc92caf 100644
--- 
a/regression-test/data/unique_with_mow_p0/partial_update/schema_change/load_without_delete_column.csv
+++ 
b/regression-test/data/unique_with_mow_p0/partial_update/schema_change/load_without_delete_column.csv
@@ -1 +1 @@
-1, 1, 1
\ No newline at end of file
+1, 2, 1, 1
\ No newline at end of file
diff --git 
a/regression-test/data/unique_with_mow_p0/partial_update/test_partial_update_schema_change.out
 
b/regression-test/data/unique_with_mow_p0/partial_update/test_partial_update_schema_change.out
index cd83800d0c..86df337471 100644
--- 
a/regression-test/data/unique_with_mow_p0/partial_update/test_partial_update_schema_change.out
+++ 
b/regression-test/data/unique_with_mow_p0/partial_update/test_partial_update_schema_change.out
@@ -15,12 +15,15 @@
 1      1       1       0       0       0       0       0       0
 
 -- !sql6 --
-1      0       0       0       0       0       0       0       0       0
+1      2       1       0       0       0       0       1       0
 
 -- !sql7 --
-1      1       1.0     0       0       0       0       0       0       0
+1      0       0       0       0       0       0       0       0       0
 
 -- !sql8 --
+1      1       1.0     0       0       0       0       0       0       0
+
+-- !sql9 --
 1
 
 -- !sql10 --
diff --git 
a/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_schema_change.groovy
 
b/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_schema_change.groovy
index 9883f19dcd..5b7e942c11 100644
--- 
a/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_schema_change.groovy
+++ 
b/regression-test/suites/unique_with_mow_p0/partial_update/test_partial_update_schema_change.groovy
@@ -212,32 +212,33 @@ suite("test_partial_update_schema_change", "p0") {
         }
     }
     qt_sql5 " select * from ${tableName} order by c0 "
-    
-    // test load data with delete column
-    // todo bug
-    // streamLoad {
-    //     table "${tableName}"
 
-    //     set 'column_separator', ','
-    //     set 'partial_columns', 'true'
-    //     set 'columns', 'c0, c1, c8'
+    // test load data with delete column, stream load will ignore the
+    // non-existing column
+    streamLoad {
+        table "${tableName}"
 
-    //     file 'schema_change/load_without_delete_column.csv'
-    //     time 10000 // limit inflight 10s
+        set 'column_separator', ','
+        set 'partial_columns', 'true'
+        set 'columns', 'c0, c1, c7, c8'
 
-    //     check { result, exception, startTime, endTime ->
-    //         if (exception != null) {
-    //             throw exception
-    //         }
-    //         // check result, which is fail for loading delete column.
-    //         log.info("Stream load result: ${result}".toString())
-    //         def json = parseJson(result)
-    //         assertEquals("fail", json.Status.toLowerCase())
-    //         assertEquals(1, json.NumberTotalRows)
-    //         assertEquals(1, json.NumberFilteredRows)
-    //         assertEquals(0, json.NumberUnselectedRows)
-    //     }
-    // }
+        file 'schema_change/load_without_delete_column.csv'
+        time 10000 // limit inflight 10s
+
+        check { result, exception, startTime, endTime ->
+            if (exception != null) {
+                throw exception
+            }
+            // check result, which is fail for loading delete column.
+            log.info("Stream load result: ${result}".toString())
+            def json = parseJson(result)
+            assertEquals("success", json.Status.toLowerCase())
+            assertEquals(1, json.NumberTotalRows)
+            assertEquals(0, json.NumberFilteredRows)
+            assertEquals(0, json.NumberUnselectedRows)
+        }
+    }
+    qt_sql6 " select * from ${tableName} order by c0 "
 
     sql """ DROP TABLE IF EXISTS ${tableName} """
 
@@ -285,7 +286,7 @@ suite("test_partial_update_schema_change", "p0") {
             assertEquals(0, json.NumberUnselectedRows)
         }
     }
-    qt_sql6 " select * from ${tableName} order by c0 "
+    qt_sql7 " select * from ${tableName} order by c0 "
     
     // schema change
     sql " ALTER table ${tableName} MODIFY COLUMN c2 double "
@@ -323,7 +324,7 @@ suite("test_partial_update_schema_change", "p0") {
             assertEquals(0, json.NumberUnselectedRows)
         }
     }
-    qt_sql7 " select * from ${tableName} order by c0 "
+    qt_sql8 " select * from ${tableName} order by c0 "
 
     sql """ DROP TABLE IF EXISTS ${tableName} """
 
@@ -363,7 +364,7 @@ suite("test_partial_update_schema_change", "p0") {
             assertEquals(0, json.NumberUnselectedRows)
         }
     }
-    qt_sql8 " select * from ${tableName} order by c0 "
+    qt_sql9 " select * from ${tableName} order by c0 "
     
     // schema change
     sql " ALTER table ${tableName} ADD COLUMN c1 int key null "
@@ -390,32 +391,30 @@ suite("test_partial_update_schema_change", "p0") {
         try_times--
     }
 
-    // test load data with all key column
-    // todo cause core
-    // streamLoad {
-    //     table "${tableName}"
+    // test load data with all key column, should fail because
+    // it don't have any value columns
+    streamLoad {
+        table "${tableName}"
 
-    //     set 'column_separator', ','
-    //     set 'partial_columns', 'true'
-    //     set 'columns', 'c0, c1'
+        set 'column_separator', ','
+        set 'partial_columns', 'true'
+        set 'columns', 'c0, c1'
 
-    //     file 'schema_change/load_with_key_column.csv'
-    //     time 10000 // limit inflight 10s
+        file 'schema_change/load_with_key_column.csv'
+        time 10000 // limit inflight 10s
 
-    //     check { result, exception, startTime, endTime ->
-    //         if (exception != null) {
-    //             throw exception
-    //         }
-    //         log.info("Stream load result: ${result}".toString())
-    //         def json = parseJson(result)
-    //         assertEquals("success", json.Status.toLowerCase())
-    //         assertEquals(1, json.NumberTotalRows)
-    //         assertEquals(0, json.NumberFilteredRows)
-    //         assertEquals(0, json.NumberUnselectedRows)
-    //     }
-    // }
-    // //check data
-    // qt_sql9 " select * from ${tableName} order by c0 "
+        check { result, exception, startTime, endTime ->
+            if (exception != null) {
+                throw exception
+            }
+            log.info("Stream load result: ${result}".toString())
+            def json = parseJson(result)
+            assertEquals("fail", json.Status.toLowerCase())
+            assertEquals(0, json.NumberTotalRows)
+            assertEquals(0, json.NumberFilteredRows)
+            assertEquals(0, json.NumberUnselectedRows)
+        }
+    }
 
     sql """ DROP TABLE IF EXISTS ${tableName} """
 


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

Reply via email to