Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: fix could not rollback when insert the table with multiple key #6077

Merged
merged 20 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changes/en-us/2.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Add changes here for all PR submitted to the 2.x branch.
- [[#6075](https://github.com/seata/seata/pull/6075)] fix missing table alias for on update column of image SQL
- [[#6086](https://github.com/seata/seata/pull/6086)] fix oracle column alias cannot find
- [[#6085](https://github.com/seata/seata/pull/6085)] fix jdk9+ compile error
- [[#6077](https://github.com/seata/seata/pull/6077)] fix could not rollback when table with multiple primary

### optimize:
- [[#6061](https://github.com/seata/seata/pull/6061)] merge the rpcMergeMessageSend threads of rm and tm and increase the thread hibernation duration
Expand Down Expand Up @@ -42,5 +43,6 @@ Thanks to these contributors for their code commits. Please report an unintended
- [wangliang181230](https://github.com/wangliang181230)
- [ggbocoder](https://github.com/ggbocoder)
- [leezongjie](https://github.com/leezongjie)
- [l81893521](https://github.com/l81893521)

Also, we receive many valuable issues, questions and advices from our community. Thanks for you all.
2 changes: 2 additions & 0 deletions changes/zh-cn/2.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- [[#6075](https://github.com/seata/seata/pull/6075)] 修复镜像SQL对于on update列没有添加表别名的问题
- [[#6086](https://github.com/seata/seata/pull/6086)] 修复oracle alias 解析异常
- [[#6085](https://github.com/seata/seata/pull/6085)] 修复jdk9+版本编译后,引入后ByteBuffer#flip NoSuchMethodError的问题
- [[#6077](https://github.com/seata/seata/pull/6077)] 修复表存在复合主键索引导致无法回滚问题

### optimize:
- [[#6061](https://github.com/seata/seata/pull/6061)] 合并rm和tm的rpcMergeMessageSend线程,增加线程休眠时长
Expand Down Expand Up @@ -41,5 +42,6 @@
- [wangliang181230](https://github.com/wangliang181230)
- [ggbocoder](https://github.com/ggbocoder)
- [leezongjie](https://github.com/leezongjie)
- [l81893521](https://github.com/l81893521)

同时,我们收到了社区反馈的很多有价值的issue和建议,非常感谢大家。
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.StringJoiner;
import java.util.TreeSet;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -193,18 +195,6 @@ protected String buildLimitCondition(WhereRecognizer recognizer, ArrayList<List<
return limitCondition;
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
/**
* Gets column name in sql.
*
* @param columnName the column name
* @return the column name in sql
*/
protected String getColumnNameInSQL(String columnName) {
String tableAlias = sqlRecognizer.getTableAlias();
return tableAlias == null ? columnName : tableAlias + "." + columnName;
}
/**
* Gets column names in sql.
*
* @param columnNames the column names
* @return
*/
protected List<String> getColumnNamesInSQLList(List<String> columnNames) {
List<String> columnNameWithTableAlias = new ArrayList<>();
for (String columnName : columnNames) {
columnNameWithTableAlias.add(this.getColumnNameInSQL(columnName));
}
return columnNameWithTableAlias;
}

* Gets column name in sql.
*
* @param columnName the column name
* @return the column name in sql
*/
protected String getColumnNameInSQL(String columnName) {
String tableAlias = sqlRecognizer.getTableAlias();
return tableAlias == null ? columnName : tableAlias + "." + columnName;
}


/**
* Gets column name with table prefix
*
Expand Down Expand Up @@ -236,10 +226,12 @@ protected List<String> getColumnNamesWithTablePrefixList(String table,String tab
/**
* Gets several column name in sql.
*
* @param table the table
* @param tableAlias the table alias
* @param columnNameList the column name
* @return the column name in sql
*/
protected String getColumnNamesInSQL(List<String> columnNameList) {
protected String getColumnNamesWithTablePrefix(String table,String tableAlias, List<String> columnNameList) {
if (CollectionUtils.isEmpty(columnNameList)) {
return null;
}
Expand All @@ -248,20 +240,43 @@ protected String getColumnNamesInSQL(List<String> columnNameList) {
if (i > 0) {
columnNamesStr.append(" , ");
}
columnNamesStr.append(getColumnNameInSQL(columnNameList.get(i)));
columnNamesStr.append(getColumnNameWithTablePrefix(table, tableAlias, columnNameList.get(i)));
}
return columnNamesStr.toString();
}

/**
* Gets column name in sql.
*
* @param columnName the column name
* @return the column name in sql
*/
protected String getColumnNameInSQL(String columnName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么要移动这段代码位置?而不是在它后面增加新方法?
Why move this piece of code instead of adding the new method after it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because reordering the type of method it looks more beautiful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because reordering the type of method it looks more beautiful.

那为什么不是将getColumnNamesInSQLList 挪到上面?
So why not move getColumnNamesInSQLList to the top?

Copy link
Contributor Author

@l81893521 l81893521 Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 method of getColumnNamesWithTablePrefix and 3 method of getColumnNameInSQL. I just put it together

String tableAlias = sqlRecognizer.getTableAlias();
return tableAlias == null ? columnName : tableAlias + "." + columnName;
}
Comment on lines +248 to +257
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* Gets column name in sql.
*
* @param columnName the column name
* @return the column name in sql
*/
protected String getColumnNameInSQL(String columnName) {
String tableAlias = sqlRecognizer.getTableAlias();
return tableAlias == null ? columnName : tableAlias + "." + columnName;
}


/**
* Gets column names in sql.
*
* @param columnNames the column names
* @return
*/
protected List<String> getColumnNamesInSQLList(List<String> columnNames) {
List<String> columnNameWithTableAlias = new ArrayList<>();
for (String columnName : columnNames) {
columnNameWithTableAlias.add(this.getColumnNameInSQL(columnName));
}
return columnNameWithTableAlias;
}

Comment on lines +259 to +272
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* Gets column names in sql.
*
* @param columnNames the column names
* @return
*/
protected List<String> getColumnNamesInSQLList(List<String> columnNames) {
List<String> columnNameWithTableAlias = new ArrayList<>();
for (String columnName : columnNames) {
columnNameWithTableAlias.add(this.getColumnNameInSQL(columnName));
}
return columnNameWithTableAlias;
}

/**
* Gets several column name in sql.
*
* @param table the table
* @param tableAlias the table alias
* @param columnNameList the column name
* @return the column name in sql
*/
protected String getColumnNamesWithTablePrefix(String table,String tableAlias, List<String> columnNameList) {
protected String getColumnNamesInSQL(List<String> columnNameList) {
if (CollectionUtils.isEmpty(columnNameList)) {
return null;
}
Expand All @@ -270,7 +285,7 @@ protected String getColumnNamesWithTablePrefix(String table,String tableAlias, L
if (i > 0) {
columnNamesStr.append(" , ");
}
columnNamesStr.append(getColumnNameWithTablePrefix(table,tableAlias, columnNameList.get(i)));
columnNamesStr.append(getColumnNameInSQL(columnNameList.get(i)));
}
return columnNamesStr.toString();
}
Expand Down Expand Up @@ -517,22 +532,24 @@ protected TableRecords buildTableRecords(Map<String, List<Object>> pkValuesMap)
}

protected List<String> getNeedColumns(String table, String tableAlias, List<String> unescapeColumns) {
List<String> needUpdateColumns = new ArrayList<>();
Set<String> needUpdateColumns = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
TableMeta tableMeta = getTableMeta(table);
if (ONLY_CARE_UPDATE_COLUMNS && CollectionUtils.isNotEmpty(unescapeColumns)) {
if (!containsPK(table, unescapeColumns)) {
List<String> pkNameList = tableMeta.getEscapePkNameList(getDbType());
if (CollectionUtils.isNotEmpty(pkNameList)) {
if (StringUtils.isNotBlank(tableAlias)) {
needUpdateColumns.add(getColumnNamesWithTablePrefix(table, tableAlias, pkNameList));
needUpdateColumns.addAll(
ColumnUtils.delEscape(getColumnNamesWithTablePrefixList(table, tableAlias, pkNameList), getDbType())
);
} else {
needUpdateColumns.add(getColumnNamesInSQL(pkNameList));
needUpdateColumns.addAll(
ColumnUtils.delEscape(getColumnNamesInSQLList(pkNameList), getDbType())
);
}
}
}
needUpdateColumns.addAll(unescapeColumns.stream()
.map(unescapeUpdateColumn -> ColumnUtils.addEscape(unescapeUpdateColumn, getDbType(), tableMeta)).collect(
Collectors.toList()));
needUpdateColumns.addAll(unescapeColumns);

// The on update xxx columns will be auto update by db, so it's also the actually updated columns
List<String> onUpdateColumns = tableMeta.getOnUpdateColumnsOnlyName();
Expand All @@ -542,18 +559,15 @@ protected List<String> getNeedColumns(String table, String tableAlias, List<Stri
.collect(Collectors.toList());
}
onUpdateColumns.removeAll(unescapeColumns);
l81893521 marked this conversation as resolved.
Show resolved Hide resolved
needUpdateColumns.addAll(onUpdateColumns.stream()
.map(onUpdateColumn -> ColumnUtils.addEscape(onUpdateColumn, getDbType(), tableMeta))
.collect(Collectors.toList()));
needUpdateColumns.addAll(onUpdateColumns);
} else {
Stream<String> allColumns = tableMeta.getAllColumns().keySet().stream();
if (StringUtils.isNotBlank(tableAlias)) {
allColumns = allColumns.map(columnName -> getColumnNameWithTablePrefix(table, tableAlias, columnName));
}
allColumns = allColumns.map(columnName -> ColumnUtils.addEscape(columnName, getDbType(), tableMeta));
allColumns.forEach(needUpdateColumns::add);
}
return needUpdateColumns;
return needUpdateColumns.parallelStream().map(column -> ColumnUtils.addEscape(column, getDbType(), tableMeta)).collect(Collectors.toList());
l81893521 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public static void init() {
} catch (Exception e) {
throw new RuntimeException("init failed");
}
String sql = "delete from t where id = 1";
String sql = "delete from table_delete_executor_test where id = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> {
Expand All @@ -96,20 +96,146 @@ public static void init() {
}

@Test
public void testBeforeImage() throws SQLException {
Assertions.assertNotNull(deleteExecutor.beforeImage());
public void testBeforeAndAfterImage() throws SQLException {
String sql = "delete from table_delete_executor_test";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithTableAlias() throws SQLException {
String sql = "delete from table_delete_executor_test t where t.id = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithTableSchema() throws SQLException {
String sql = "delete from seata.table_delete_executor_test where id = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

String sql = "delete from t";
TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithTableSchemaAndTableAlias() throws SQLException {
String sql = "delete from seata.table_delete_executor_test t where t.id = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);
Assertions.assertNotNull(deleteExecutor.beforeImage());

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testAfterImage() throws SQLException {
TableRecords tableRecords = deleteExecutor.beforeImage();
Assertions.assertEquals(0, deleteExecutor.afterImage(tableRecords).size());
public void testBeforeAndAfterImageWithTableSchemaQuote() throws SQLException {
String sql = "delete from `seata`.table_delete_executor_test where id = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithTableSchemaAndTableNameQuote() throws SQLException {
String sql = "delete from seata.`table_delete_executor_test` where id = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithTableSchemaQuoteAndTableNameQuote() throws SQLException {
String sql = "delete from `seata`.`table_delete_executor_test` where id = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithColumnQuote() throws SQLException {
String sql = "delete from table_delete_executor_test where `id` = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithUpperColumn() throws SQLException {
String sql = "delete from table_delete_executor_test where ID = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithTableAliasAndUpperColumn() throws SQLException {
String sql = "delete from table_delete_executor_test t where t.ID = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

@Test
public void testBeforeAndAfterImageWithKeyword() throws SQLException {
String sql = "delete from table_delete_executor_test where `or` = 1";
List<SQLStatement> asts = SQLUtils.parseStatements(sql, JdbcConstants.MYSQL);
MySQLDeleteRecognizer recognizer = new MySQLDeleteRecognizer(sql, asts.get(0));
deleteExecutor = new DeleteExecutor(statementProxy, (statement, args) -> null, recognizer);

TableRecords beforeImage = deleteExecutor.beforeImage();
TableRecords afterImage = deleteExecutor.afterImage(beforeImage);
Assertions.assertNotNull(beforeImage);
Assertions.assertNotNull(afterImage);
}

}
Loading
Loading