xiaokang commented on code in PR #42680: URL: https://github.com/apache/doris/pull/42680#discussion_r1821699791
########## regression-test/suites/query_p0/system/test_storage_page_size.groovy: ########## @@ -0,0 +1,63 @@ +// 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. + +import org.codehaus.groovy.runtime.IOGroovyMethods + +suite ("test_storage_page_size") { + + sql """ DROP TABLE IF EXISTS table_1; """ + + sql """ + create table table_1( + k1 int not null, + k2 int not null, + k3 bigint null, + k4 varchar(100) null + ) + duplicate key (k1) + distributed BY random buckets 1 + properties("replication_num" = "1"); + """ + + test { + sql "show create table table_1;" + check { result, exception, startTime, endTime -> + assertTrue(result[0][1].contains("\"storage_page_size\" = \"65536\"")) + } + } + + sql """ DROP TABLE IF EXISTS table_2; """ + + sql """ + create table table_2 ( Review Comment: add test for more values like <<min, min-1, min, min+1, min~max, max-1,max,max+1, >>max ########## be/src/olap/rowset/segment_v2/options.h: ########## @@ -25,6 +25,7 @@ namespace segment_v2 { static constexpr size_t DEFAULT_PAGE_SIZE = 1024 * 1024; // default size: 1M constexpr long ROW_STORE_PAGE_SIZE_DEFAULT_VALUE = 16384; // default row store page size: 16KB +static constexpr int64_t STORAGE_PAGE_SIZE_DEFAULT_VALUE = 65536; Review Comment: use size_t ########## be/src/olap/rowset/segment_v2/segment_writer.cpp: ########## @@ -261,6 +261,10 @@ Status SegmentWriter::_create_column_writer(uint32_t cid, const TabletColumn& co opts.data_page_size = (page_size > 0) ? page_size : segment_v2::ROW_STORE_PAGE_SIZE_DEFAULT_VALUE; } + int64_t storage_page_size = _tablet_schema->storage_page_size(); Review Comment: Change code for normal storage_page_size before row_store_page_size to avoid logic problem, since storage_page_size is default value and row_store_page_size is special value, special value should be processed after default value. ########## fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java: ########## @@ -1075,6 +1078,24 @@ public static long analyzeRowStorePageSize(Map<String, String> properties) throw return rowStorePageSize; } + public static long analyzeStoragePageSize(Map<String, String> properties) throws AnalysisException { + long storagePageSize = STORAGE_PAGE_SIZE_DEFAULT_VALUE; + if (properties != null && properties.containsKey(PROPERTIES_STORAGE_PAGE_SIZE)) { + String storagePageSizeStr = properties.get(PROPERTIES_STORAGE_PAGE_SIZE); + try { + storagePageSize = Long.parseLong(storagePageSizeStr); + } catch (NumberFormatException e) { + throw new AnalysisException("Invalid storage page size: " + storagePageSizeStr); + } + if (storagePageSize < 65536 || storagePageSize > 1048576) { Review Comment: allow for bigger range [4K, 10MB] -- 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
