This is an automated email from the ASF dual-hosted git repository. kxiao pushed a commit to branch branch-2.0 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 2aa4428e9c5cdb132a5aacfc21c5ef48de6a27f4 Author: Jack Drogon <[email protected]> AuthorDate: Sat Jul 29 08:27:27 2023 +0800 [enhancement](binlog) CreateTable inherit db binlog && Add some checks (#22293) --- .../apache/doris/alter/SchemaChangeHandler.java | 1 - .../org/apache/doris/analysis/CreateTableStmt.java | 3 + .../org/apache/doris/catalog/BinlogConfig.java | 4 + .../apache/doris/datasource/InternalCatalog.java | 14 ++++ .../apache/doris/analysis/CreateTableStmtTest.java | 8 +- .../test_create_table_with_binlog_config.groovy | 88 ++++++++++++++++++++++ 6 files changed, 113 insertions(+), 5 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java index 07770833f5..ff2fcc3fd6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java @@ -2799,7 +2799,6 @@ public class SchemaChangeHandler extends AlterHandler { public boolean updateBinlogConfig(Database db, OlapTable olapTable, List<AlterClause> alterClauses) throws DdlException, UserException { - // TODO(Drogon): check olapTable read binlog thread safety List<Partition> partitions = Lists.newArrayList(); BinlogConfig oldBinlogConfig; BinlogConfig newBinlogConfig; diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java index 64409d3b4a..8aab65cd6f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java @@ -269,6 +269,9 @@ public class CreateTableStmt extends DdlStmt { } public Map<String, String> getProperties() { + if (this.properties == null) { + this.properties = Maps.newHashMap(); + } return this.properties; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/BinlogConfig.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/BinlogConfig.java index f0f4efbc46..2f35a8d59c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/BinlogConfig.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/BinlogConfig.java @@ -64,6 +64,10 @@ public class BinlogConfig implements Writable { } public void mergeFromProperties(Map<String, String> properties) { + if (properties == null) { + return; + } + if (properties.containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_ENABLE)) { enable = Boolean.parseBoolean(properties.get( PropertyAnalyzer.PROPERTIES_BINLOG_ENABLE)); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 09a438846c..a4ada8e761 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -1875,6 +1875,20 @@ public class InternalCatalog implements CatalogIf<Database> { String tableName = stmt.getTableName(); LOG.debug("begin create olap table: {}", tableName); + BinlogConfig dbBinlogConfig; + db.readLock(); + try { + dbBinlogConfig = new BinlogConfig(db.getBinlogConfig()); + } finally { + db.readUnlock(); + } + BinlogConfig createTableBinlogConfig = new BinlogConfig(dbBinlogConfig); + createTableBinlogConfig.mergeFromProperties(stmt.getProperties()); + if (dbBinlogConfig.isEnable() && !createTableBinlogConfig.isEnable()) { + throw new DdlException("Cannot create table with binlog disabled when database binlog enable"); + } + stmt.getProperties().putAll(createTableBinlogConfig.toProperties()); + // get keys type KeysDesc keysDesc = stmt.getKeysDesc(); Preconditions.checkNotNull(keysDesc); diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java index 9138248c01..d9e3b0019e 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java @@ -111,7 +111,7 @@ public class CreateTableStmtTest { stmt.analyze(analyzer); Assert.assertEquals("testCluster:db1", stmt.getDbName()); Assert.assertEquals("table1", stmt.getTableName()); - Assert.assertNull(stmt.getProperties()); + Assert.assertTrue(stmt.getProperties() == null || stmt.getProperties().isEmpty()); } @Test @@ -121,7 +121,7 @@ public class CreateTableStmtTest { stmt.analyze(analyzer); Assert.assertEquals("testCluster:db1", stmt.getDbName()); Assert.assertEquals("table1", stmt.getTableName()); - Assert.assertNull(stmt.getProperties()); + Assert.assertTrue(stmt.getProperties() == null || stmt.getProperties().isEmpty()); Assert.assertTrue(stmt.toSql().contains("DISTRIBUTED BY RANDOM\nBUCKETS 6")); } @@ -242,7 +242,7 @@ public class CreateTableStmtTest { stmt.analyze(analyzer); Assert.assertEquals("testCluster:db1", stmt.getDbName()); Assert.assertEquals("table1", stmt.getTableName()); - Assert.assertNull(stmt.getProperties()); + Assert.assertTrue(stmt.getProperties() == null || stmt.getProperties().isEmpty()); Assert.assertTrue(stmt.toSql() .contains("rollup( `index1` (`col1`, `col2`) FROM `table1`, `index2` (`col2`, `col3`) FROM `table1`)")); } @@ -256,7 +256,7 @@ public class CreateTableStmtTest { Assert.assertEquals("testDb", stmt.getDbName()); Assert.assertEquals("table1", stmt.getTableName()); Assert.assertNull(stmt.getPartitionDesc()); - Assert.assertNull(stmt.getProperties()); + Assert.assertTrue(stmt.getProperties() == null || stmt.getProperties().isEmpty()); } @Test(expected = AnalysisException.class) diff --git a/regression-test/suites/ccr_syncer_p0/test_create_table_with_binlog_config.groovy b/regression-test/suites/ccr_syncer_p0/test_create_table_with_binlog_config.groovy new file mode 100644 index 0000000000..0065ede1c6 --- /dev/null +++ b/regression-test/suites/ccr_syncer_p0/test_create_table_with_binlog_config.groovy @@ -0,0 +1,88 @@ +// 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_create_table_with_binlog_config") { + sql "drop database if exists test_table_binlog" + + sql """ + create database test_table_binlog + """ + result = sql "show create database test_table_binlog" + logger.info("${result}") + + // Case 1: database disable binlog, create table with binlog disable + sql """ + CREATE TABLE test_table_binlog.t1 ( k1 INT ) ENGINE = olap DISTRIBUTED BY HASH(k1) BUCKETS 3 PROPERTIES ( "replication_num" = "1", "binlog.enable" = "false" ); + """ + result = sql "show create table test_table_binlog.t1" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"binlog.enable" = "false"')) + sql """ + drop table if exists test_table_binlog.t1 + """ + + // Case 2: database disable binlog, create table with binlog enable + sql """ + CREATE TABLE test_table_binlog.t1 ( k1 INT ) ENGINE = olap DISTRIBUTED BY HASH(k1) BUCKETS 3 PROPERTIES ( "replication_num" = "1", "binlog.enable" = "true" ); + """ + result = sql "show create table test_table_binlog.t1" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"binlog.enable" = "true"')) + sql """ + drop table if exists test_table_binlog.t1 + """ + + // Case 3: database enable binlog, create table with binlog disable + sql """ + alter database test_table_binlog set properties ("binlog.enable" = "true") + """ + assertThrows(Exception.class, { + sql """ + CREATE TABLE test_table_binlog.t1 ( k1 INT ) ENGINE = olap DISTRIBUTED BY HASH(k1) BUCKETS 3 PROPERTIES ( "replication_num" = "1", "binlog.enable" = "false" ); + """ + }) + sql """ + drop table if exists test_table_binlog.t1 + """ + + // Case 4: database enable binlog, create table with binlog enable + sql """ + alter database test_table_binlog set properties ("binlog.enable" = "true") + """ + sql """ + CREATE TABLE test_table_binlog.t1 ( k1 INT ) ENGINE = olap DISTRIBUTED BY HASH(k1) BUCKETS 3 PROPERTIES ( "replication_num" = "1", "binlog.enable" = "true" ); + """ + result = sql "show create table test_table_binlog.t1" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"binlog.enable" = "true"')) + sql """ + drop table if exists test_table_binlog.t1 + """ + + // Case 5: database enable binlog, create table inherit database binlog config + sql """ + CREATE TABLE test_table_binlog.t1 ( k1 INT ) ENGINE = olap DISTRIBUTED BY HASH(k1) BUCKETS 3 PROPERTIES ( "replication_num" = "1" ); + """ + result = sql "show create table test_table_binlog.t1" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"binlog.enable" = "true"')) + sql """ + drop table if exists test_table_binlog.t1 + """ + + sql "drop database if exists test_table_binlog" +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
