From 5ee13de752f429fa49944da71625592b8ec0b679 Mon Sep 17 00:00:00 2001 From: "wuwenqi.wwq" Date: Wed, 1 Nov 2023 10:41:43 +0800 Subject: [PATCH] fix foreign key bugs --- .../apache/calcite/sql/SqlCreateTable.java | 18 ++-- .../ddl/job/task/cdc/CdcMarkUtil.java | 20 +++-- .../job/validator/ForeignKeyValidator.java | 86 +++++++++++++------ .../handler/ddl/LogicalCommonDdlHandler.java | 43 ++++++++++ .../optimizer/context/DdlContext.java | 12 +++ .../MoveDatabaseWithForeignKeyTest.java | 6 +- 6 files changed, 143 insertions(+), 42 deletions(-) diff --git a/polardbx-calcite/src/main/java/org/apache/calcite/sql/SqlCreateTable.java b/polardbx-calcite/src/main/java/org/apache/calcite/sql/SqlCreateTable.java index 034875d6e..6d9b2c594 100644 --- a/polardbx-calcite/src/main/java/org/apache/calcite/sql/SqlCreateTable.java +++ b/polardbx-calcite/src/main/java/org/apache/calcite/sql/SqlCreateTable.java @@ -19,8 +19,8 @@ import com.alibaba.polardbx.common.ArchiveMode; import com.alibaba.polardbx.common.Engine; import com.alibaba.polardbx.common.TddlConstants; -import com.alibaba.polardbx.common.exception.TddlRuntimeException; import com.alibaba.polardbx.common.ddl.foreignkey.ForeignKeyData; +import com.alibaba.polardbx.common.exception.TddlRuntimeException; import com.alibaba.polardbx.common.utils.CaseInsensitive; import com.alibaba.polardbx.common.utils.GeneralUtil; import com.alibaba.polardbx.common.utils.TStringUtil; @@ -113,8 +113,6 @@ import static com.alibaba.polardbx.common.TddlConstants.IMPLICIT_COL_NAME; import static com.alibaba.polardbx.common.TddlConstants.IMPLICIT_KEY_NAME; import static com.alibaba.polardbx.common.TddlConstants.UGSI_PK_INDEX_NAME; -import static com.alibaba.polardbx.common.exception.code.ErrorCode.ERR_CREATE_SELECT_FUNCTION_ALIAS; -import static com.alibaba.polardbx.common.exception.code.ErrorCode.ERR_CREATE_SELECT_WITH_GSI; import static com.alibaba.polardbx.common.exception.code.ErrorCode.ERR_CREATE_SELECT_WITH_OSS; /** @@ -370,9 +368,8 @@ public SqlCreateTable(SqlParserPos pos, boolean replace, boolean ifNotExists, Sq List> spatialKeys, List> foreignKeys, List checks, SqlIdentifier primaryKeyConstraint, boolean hasPrimaryKeyConstraint, SqlNode sqlPartition, - SqlNode localPartition, - SqlNode tableGroupName, - SqlNode joinGroupName) { + SqlNode localPartition, SqlNode tableGroupName, SqlNode joinGroupName, + List addedForeignKeys, String defaultCharset, String defaultCollation) { super(OPERATOR, pos, replace, ifNotExists); this.name = name; this.likeTableName = likeTableName; @@ -405,6 +402,9 @@ public SqlCreateTable(SqlParserPos pos, boolean replace, boolean ifNotExists, Sq this.localPartition = localPartition; this.tableGroupName = tableGroupName; this.joinGroupName = joinGroupName; + this.addedForeignKeys = addedForeignKeys; + this.defaultCharset = defaultCharset; + this.defaultCollation = defaultCollation; } public boolean shouldLoad() { @@ -922,7 +922,10 @@ public SqlCreateTable clone(SqlParserPos pos) { sqlPartition, localPartition, tableGroupName, - joinGroupName); + joinGroupName, + addedForeignKeys, + defaultCharset, + defaultCollation); ret.setEngine(engine); ret.setDBPartition(DbPartition); return ret; @@ -2533,3 +2536,4 @@ public boolean isCharType() { } // End SqlCreateTable.java + diff --git a/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/task/cdc/CdcMarkUtil.java b/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/task/cdc/CdcMarkUtil.java index f1c3bf828..d65319958 100644 --- a/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/task/cdc/CdcMarkUtil.java +++ b/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/task/cdc/CdcMarkUtil.java @@ -59,6 +59,8 @@ public static Map buildExtendParameter(ExecutionContext executio parameter.put(ICdcManager.CDC_ORIGINAL_DDL, ""); if (isUseOriginalDDL(executionContext)) { parameter.put(ICdcManager.CDC_ORIGINAL_DDL, executionContext.getDdlContext().getDdlStmt()); + } else if (isUseFkOriginalDDL(executionContext)) { + parameter.put(ICdcManager.CDC_ORIGINAL_DDL, executionContext.getDdlContext().getForeignKeyOriginalSql()); } return parameter; } @@ -66,7 +68,6 @@ public static Map buildExtendParameter(ExecutionContext executio private static boolean isUseOriginalDDL(ExecutionContext executionContext) { Map parameter = executionContext.getExtraCmds(); String useOriginalDDL = (String) parameter.get(ICdcManager.USE_ORGINAL_DDL); - String foreignKeysDdl = (String) parameter.get(ICdcManager.FOREIGN_KEYS_DDL); if (executionContext.getDdlContext() == null || StringUtils.isEmpty( @@ -74,10 +75,19 @@ private static boolean isUseOriginalDDL(ExecutionContext executionContext) { return false; } - if (StringUtils.equalsIgnoreCase("true", useOriginalDDL) || - StringUtils.equalsIgnoreCase("true", foreignKeysDdl)) { - return true; + return StringUtils.equalsIgnoreCase("true", useOriginalDDL); + } + + private static boolean isUseFkOriginalDDL(ExecutionContext executionContext) { + Map parameter = executionContext.getExtraCmds(); + String foreignKeysDdl = (String) parameter.get(ICdcManager.FOREIGN_KEYS_DDL); + + if (executionContext.getDdlContext() == null || + StringUtils.isEmpty( + executionContext.getDdlContext().getDdlStmt())) { + return false; } - return false; + + return StringUtils.equalsIgnoreCase("true", foreignKeysDdl); } } diff --git a/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/validator/ForeignKeyValidator.java b/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/validator/ForeignKeyValidator.java index 1b2e86ca9..ac0326e91 100644 --- a/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/validator/ForeignKeyValidator.java +++ b/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/ddl/job/validator/ForeignKeyValidator.java @@ -17,7 +17,6 @@ package com.alibaba.polardbx.executor.ddl.job.validator; import com.alibaba.polardbx.common.Engine; -import com.alibaba.polardbx.common.charset.CharsetName; import com.alibaba.polardbx.common.ddl.foreignkey.ForeignKeyData; import com.alibaba.polardbx.common.exception.TddlRuntimeException; import com.alibaba.polardbx.common.exception.code.ErrorCode; @@ -26,7 +25,6 @@ import com.alibaba.polardbx.druid.util.StringUtils; import com.alibaba.polardbx.gms.metadb.limit.LimitValidator; import com.alibaba.polardbx.gms.topology.DbInfoManager; -import com.alibaba.polardbx.gms.topology.DbInfoRecord; import com.alibaba.polardbx.optimizer.config.table.ColumnMeta; import com.alibaba.polardbx.optimizer.config.table.GeneratedColumnUtil; import com.alibaba.polardbx.optimizer.config.table.IndexMeta; @@ -34,6 +32,8 @@ import com.alibaba.polardbx.optimizer.context.ExecutionContext; import com.alibaba.polardbx.optimizer.core.TddlRelDataTypeSystemImpl; import com.alibaba.polardbx.optimizer.core.TddlTypeFactoryImpl; +import com.alibaba.polardbx.optimizer.utils.DdlCharsetInfo; +import com.alibaba.polardbx.optimizer.utils.DdlCharsetInfoUtil; import com.alibaba.polardbx.optimizer.utils.ForeignKeyUtils; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; @@ -41,17 +41,16 @@ import org.apache.calcite.sql.SqlAlterTable; import org.apache.calcite.sql.SqlAlterTableDropIndex; import org.apache.calcite.sql.SqlCall; -import org.apache.calcite.sql.SqlCollation; import org.apache.calcite.sql.SqlColumnDeclaration; import org.apache.calcite.sql.SqlCreateTable; import org.apache.calcite.sql.SqlDataTypeSpec; import org.apache.calcite.sql.SqlIdentifier; import org.apache.calcite.sql.SqlIndexDefinition; import org.apache.calcite.sql.SqlModifyColumn; +import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.calcite.util.Pair; -import java.nio.charset.Charset; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -65,8 +64,10 @@ public class ForeignKeyValidator { - public static void validateFkConstraints(SqlCreateTable sqlCreateTable, String schemaName, String tableName, + public static void validateFkConstraints(SqlCreateTable sqlCreateTableOrigin, String schemaName, String tableName, ExecutionContext executionContext) { + SqlCreateTable sqlCreateTable = getCharsetAndCollation(sqlCreateTableOrigin, schemaName, executionContext); + final boolean checkForeignKey = executionContext.foreignKeyChecks(); @@ -228,30 +229,16 @@ public static void validateFkConstraints(SqlCreateTable sqlCreateTable, String s } } - DbInfoRecord dbInfoRecord = DbInfoManager.getInstance().getDbInfo(schemaName); - String charset = Optional.ofNullable(sqlCreateTable.getDefaultCharset()) - .orElse(dbInfoRecord == null ? null : dbInfoRecord.charset); - Charset tableCharset = Optional.ofNullable(charset) - .map(CharsetName::convertStrToJavaCharset) - .orElseGet( - () -> CharsetName.defaultCharset().toJavaCharset() - ); - if (!data.refTableName.equalsIgnoreCase(tableName)) { // charset and collation must be same - String collation = Optional.ofNullable(sqlCreateTable.getDefaultCollation()) - .orElse(dbInfoRecord == null ? null : dbInfoRecord.collation); - if (collation == null) { - SqlCollation tableCollation = new SqlCollation(tableCharset, sqlCreateTable.getDefaultCollation(), - SqlCollation.Coercibility.IMPLICIT); - collation = tableCollation.getCollationName(); - } - if (!StringUtils.equalsIgnoreCase(tableCharset.name(), referringTableMeta.getDefaultCharset())) { + if (!StringUtils.equalsIgnoreCase(sqlCreateTable.getDefaultCharset(), + referringTableMeta.getDefaultCharset())) { throw new TddlRuntimeException(ErrorCode.ERR_ADD_FK_CHARSET_COLLATION, schemaName, tableName, data.refSchema, data.refTableName); } - if (!StringUtils.equalsIgnoreCase(collation, referringTableMeta.getDefaultCollation())) { + if (!StringUtils.equalsIgnoreCase(sqlCreateTable.getDefaultCollation(), + referringTableMeta.getDefaultCollation())) { throw new TddlRuntimeException(ErrorCode.ERR_ADD_FK_CHARSET_COLLATION, schemaName, tableName, data.refSchema, data.refTableName); } @@ -282,10 +269,8 @@ public static void validateFkConstraints(SqlCreateTable sqlCreateTable, String s RelDataType type = def.getDataType().deriveType(factory, nullable); if (charSetName == null && SqlTypeUtil.inCharFamily(type)) { - SqlCollation collation = new SqlCollation(tableCharset, sqlCreateTable.getDefaultCollation(), - SqlCollation.Coercibility.IMPLICIT); - charSetName = tableCharset.name(); - collationName = collation.getCollationName(); + charSetName = sqlCreateTable.getDefaultCharset(); + collationName = sqlCreateTable.getDefaultCollation(); } if (!columnTypeName.equals( @@ -749,4 +734,51 @@ private static void checkColumnType(SqlModifyColumn alterItem, } } } + + private static SqlCreateTable getCharsetAndCollation(SqlCreateTable sqlCreateTableOrigin, String schemaName, + ExecutionContext executionContext) { + SqlCreateTable sqlCreateTable = sqlCreateTableOrigin.clone(SqlParserPos.ZERO); + String defaultCharset = sqlCreateTable.getDefaultCharset(); + String defaultCollation = sqlCreateTable.getDefaultCollation(); + String charset; + String collation; + StringBuilder builder = new StringBuilder(); + charset = DbInfoManager.getInstance().getDbChartSet(schemaName); + collation = DbInfoManager.getInstance().getDbCollation(schemaName); + + if (StringUtils.isEmpty(charset) || StringUtils.isEmpty(collation)) { + /** + * For some unit-test, its dbInfo is mock,so its charset & collation maybe null + */ + // Fetch server default collation + DdlCharsetInfo serverDefaultCharsetInfo = + DdlCharsetInfoUtil.fetchServerDefaultCharsetInfo(executionContext, true); + + // Fetch db collation by charset & charset defined by user and server default collation + DdlCharsetInfo createDbCharInfo = + DdlCharsetInfoUtil.decideDdlCharsetInfo(executionContext, serverDefaultCharsetInfo.finalCharset, + serverDefaultCharsetInfo.finalCollate, + charset, collation, true); + charset = createDbCharInfo.finalCharset; + collation = createDbCharInfo.finalCollate; + } + + // Fetch tbl collation by charset & charset defined by user and db collation + DdlCharsetInfo createTbCharInfo = + DdlCharsetInfoUtil.decideDdlCharsetInfo(executionContext, charset, collation, + defaultCharset, defaultCollation, true); + + if (defaultCharset == null && defaultCollation == null && createTbCharInfo.finalCharset != null) { + builder.append(" CHARSET `").append(createTbCharInfo.finalCharset.toLowerCase()).append("`"); + sqlCreateTable.setDefaultCharset(createTbCharInfo.finalCharset.toLowerCase()); + } + + if (defaultCollation == null && createTbCharInfo.finalCollate != null) { + builder.append(" COLLATE `").append(createTbCharInfo.finalCollate.toLowerCase()).append("`"); + sqlCreateTable.setDefaultCollation(createTbCharInfo.finalCollate.toLowerCase()); + } + + return sqlCreateTable; + } } + diff --git a/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/handler/ddl/LogicalCommonDdlHandler.java b/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/handler/ddl/LogicalCommonDdlHandler.java index 61b8de5a0..c1e12ff47 100644 --- a/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/handler/ddl/LogicalCommonDdlHandler.java +++ b/polardbx-executor/src/main/java/com/alibaba/polardbx/executor/handler/ddl/LogicalCommonDdlHandler.java @@ -68,6 +68,7 @@ import com.alibaba.polardbx.optimizer.core.datatype.DataTypes; import com.alibaba.polardbx.optimizer.core.rel.dal.PhyShow; import com.alibaba.polardbx.optimizer.core.rel.ddl.BaseDdlOperation; +import com.alibaba.polardbx.optimizer.core.rel.ddl.LogicalAlterTable; import com.alibaba.polardbx.optimizer.core.rel.ddl.LogicalCreateTable; import com.alibaba.polardbx.optimizer.core.row.Row; import com.alibaba.polardbx.optimizer.parse.FastsqlParser; @@ -75,9 +76,11 @@ import com.alibaba.polardbx.optimizer.partition.PartitionInfoManager; import com.alibaba.polardbx.optimizer.partition.common.PartitionLocation; import com.alibaba.polardbx.optimizer.rule.TddlRuleManager; +import com.alibaba.polardbx.optimizer.utils.ForeignKeyUtils; import com.alibaba.polardbx.optimizer.utils.RelUtils; import com.alibaba.polardbx.rule.model.TargetDB; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.sql.SqlAddForeignKey; import org.apache.calcite.sql.SqlAddPrimaryKey; import org.apache.calcite.sql.SqlAlterSpecification; import org.apache.calcite.sql.SqlAlterTable; @@ -86,9 +89,12 @@ import org.apache.calcite.sql.SqlCreateTable; import org.apache.calcite.sql.SqlDropPrimaryKey; import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlModifyColumn; import org.apache.calcite.sql.SqlShowCreateTable; +import org.apache.calcite.sql.dialect.MysqlSqlDialect; import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.pretty.SqlPrettyWriter; import org.apache.calcite.util.EqualsContext; import org.apache.calcite.util.Litmus; import org.apache.commons.collections.CollectionUtils; @@ -223,6 +229,8 @@ protected void initDdlContext(BaseDdlOperation logicalDdlPlan, ExecutionContext DdlContext ddlContext = DdlContext.create(schemaName, objectName, ddlType, executionContext); + rewriteOriginSqlWithForeignKey(logicalDdlPlan, ddlContext, schemaName, objectName); + executionContext.setDdlContext(ddlContext); } @@ -522,4 +530,39 @@ protected boolean isAvailableForRecycleBin(String tableName, ExecutionContext ex recycleBin != null && !recycleBin.hasForeignConstraint(appName, tableName); } + protected void rewriteOriginSqlWithForeignKey(BaseDdlOperation logicalDdlPlan, DdlContext ddlContext, + String schemaName, String tableName) { + // rewrite origin sql for different naming behaviours in 5.7 & 8.0 + boolean createTableWithFk = logicalDdlPlan.getDdlType() == DdlType.CREATE_TABLE + && !((LogicalCreateTable) logicalDdlPlan).getSqlCreateTable().getAddedForeignKeys().isEmpty(); + boolean alterTableWithFk = logicalDdlPlan.getDdlType() == DdlType.ALTER_TABLE + && ((LogicalAlterTable) logicalDdlPlan).getSqlAlterTable().getAlters().get(0).getKind() + == SqlKind.ADD_FOREIGN_KEY; + if (createTableWithFk) { + ddlContext.setForeignKeyOriginalSql( + ((LogicalCreateTable) logicalDdlPlan).getSqlCreateTable().toString()); + } else if (alterTableWithFk) { + final SqlAlterTable sqlTemplate = ((LogicalAlterTable) logicalDdlPlan).getSqlAlterTable(); + + SqlAddForeignKey sqlAddForeignKey = + (SqlAddForeignKey) ((LogicalAlterTable) logicalDdlPlan).getSqlAlterTable().getAlters().get(0); + // create foreign key constraints symbol + String symbol = + ForeignKeyUtils.getForeignKeyConstraintName(schemaName, tableName); + if (sqlAddForeignKey.getConstraint() == null) { + sqlAddForeignKey.setConstraint(new SqlIdentifier(SQLUtils.normalizeNoTrim(symbol), SqlParserPos.ZERO)); + } + SqlPrettyWriter writer = new SqlPrettyWriter(MysqlSqlDialect.DEFAULT); + writer.setAlwaysUseParentheses(true); + writer.setSelectListItemsOnSeparateLines(false); + writer.setIndentation(0); + final int leftPrec = sqlTemplate.getOperator().getLeftPrec(); + final int rightPrec = sqlTemplate.getOperator().getRightPrec(); + sqlTemplate.getAlters().clear(); + sqlTemplate.getAlters().add(sqlAddForeignKey); + sqlTemplate.unparse(writer, leftPrec, rightPrec, true); + + ddlContext.setForeignKeyOriginalSql(writer.toSqlString().getSql()); + } + } } diff --git a/polardbx-optimizer/src/main/java/com/alibaba/polardbx/optimizer/context/DdlContext.java b/polardbx-optimizer/src/main/java/com/alibaba/polardbx/optimizer/context/DdlContext.java index f6c159fd8..25f3b0dcd 100644 --- a/polardbx-optimizer/src/main/java/com/alibaba/polardbx/optimizer/context/DdlContext.java +++ b/polardbx-optimizer/src/main/java/com/alibaba/polardbx/optimizer/context/DdlContext.java @@ -101,6 +101,9 @@ public class DdlContext { private String sqlMode; + // rewrite origin sql for different fk naming behaviours in 5.7 & 8.0 + private String foreignKeyOriginalSql; + public static DdlContext create(String schemaName, String objectName, DdlType ddlType, ExecutionContext executionContext) { DdlContext ddlContext = new DdlContext(); @@ -210,6 +213,7 @@ public DdlContext copy() { res.setEncoding(getEncoding()); res.setTimeZone(getTimeZone()); res.setParentDdlContext(getParentDdlContext()); + res.setForeignKeyOriginalSql(getForeignKeyOriginalSql()); return res; } @@ -516,4 +520,12 @@ public DdlState getRollbackPausedPolicy() { public void setRollbackPausedPolicy(DdlState rollbackPausedPolicy) { this.rollbackPausedPolicy = rollbackPausedPolicy; } + + public void setForeignKeyOriginalSql(String foreignKeyOriginalSql) { + this.foreignKeyOriginalSql = foreignKeyOriginalSql; + } + + public String getForeignKeyOriginalSql() { + return this.foreignKeyOriginalSql; + } } diff --git a/polardbx-test/src/test/java/com/alibaba/polardbx/qatest/ddl/sharding/movedatabase/MoveDatabaseWithForeignKeyTest.java b/polardbx-test/src/test/java/com/alibaba/polardbx/qatest/ddl/sharding/movedatabase/MoveDatabaseWithForeignKeyTest.java index 9ac743b59..d6180a15a 100644 --- a/polardbx-test/src/test/java/com/alibaba/polardbx/qatest/ddl/sharding/movedatabase/MoveDatabaseWithForeignKeyTest.java +++ b/polardbx-test/src/test/java/com/alibaba/polardbx/qatest/ddl/sharding/movedatabase/MoveDatabaseWithForeignKeyTest.java @@ -25,12 +25,12 @@ public class MoveDatabaseWithForeignKeyTest extends MoveDatabaseBaseTest { //delete hint SHARE_STORAGE_MODE=true,SCALE_OUT_DROP_DATABASE_AFTER_SWITCH_DATASOURCE=true because there are always 2 dn in k8s public String scaleOutHint = - " /*+TDDL:CMD_EXTRA(SHARE_STORAGE_MODE=true)*/ "; + ""; public String scaleOutHint2 = "/*+TDDL:CMD_EXTRA(" + "PHYSICAL_TABLE_START_SPLIT_SIZE = 100, PHYSICAL_TABLE_BACKFILL_PARALLELISM = 2, " - + "ENABLE_SLIDE_WINDOW_BACKFILL = true, SLIDE_WINDOW_SPLIT_SIZE = 2, SLIDE_WINDOW_TIME_INTERVAL = 1000, SHARE_STORAGE_MODE=true)*/"; + + "ENABLE_SLIDE_WINDOW_BACKFILL = true, SLIDE_WINDOW_SPLIT_SIZE = 2, SLIDE_WINDOW_TIME_INTERVAL = 1000)*/"; public String createChild2Sql = "create table `%s` (`a` int(11) primary key auto_increment, `b` int(11), `c` timestamp DEFAULT CURRENT_TIMESTAMP, " @@ -68,7 +68,7 @@ public MoveDatabaseWithForeignKeyTest(Boolean useParallelBackfill) { void doReCreateDatabase() { doClearDatabase(); - String createDbHint = "/*+TDDL({\"extra\":{\"SHARD_DB_COUNT_EACH_STORAGE_INST_FOR_STMT\":\"4\"}})*/"; + String createDbHint = "/*+TDDL({\"extra\":{\"SHARD_DB_COUNT_EACH_STORAGE_INST_FOR_STMT\":\"2\"}})*/"; String tddlSql = "use information_schema"; JdbcUtil.executeUpdate(tddlConnection, tddlSql); tddlSql = createDbHint + "create database " + dataBaseName + " partition_mode = 'drds'";