Skip to content

Commit

Permalink
bugfix: Fixed that the same record has different lowkeys due to mixed…
Browse files Browse the repository at this point in the history
… case of table names (#6678)
  • Loading branch information
GoodBoyCoder authored Jul 16, 2024
1 parent e8ea2cc commit 1c0a442
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 16 deletions.
4 changes: 3 additions & 1 deletion changes/en-us/2.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Add changes here for all PR submitted to the 2.x branch.
- [[#6642](https://github.com/apache/incubator-seata/pull/6642)] codecov token not found
- [[#6661](https://github.com/apache/incubator-seata/pull/6661)] fix `tableMeta` cache scheduled refresh issue
- [[#6668](https://github.com/apache/incubator-seata/pull/6668)] thread safety issue when adding and removing instances
- [[#6678](https://github.com/apache/incubator-seata/pull/6678)] fix the same record has different lowkeys due to mixed case of table names yesterday

### optimize:
- [[#6499](https://github.com/apache/incubator-seata/pull/6499)] split the task thread pool for committing and rollbacking statuses
Expand Down Expand Up @@ -56,6 +57,7 @@ Thanks to these contributors for their code commits. Please report an unintended
- [xjlgod](https://github.com/xjlgod)
- [xingfudeshi](https://github.com/xingfudeshi)
- [wuwen5](https://github.com/wuwen5)
- [iAmClever(https://github.com/iAmClever)
- [iAmClever](https://github.com/iAmClever)
- [GoodBoyCoder](https://github.com/GoodBoyCoder)

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 @@ -15,6 +15,7 @@
- [[#6642](https://github.com/apache/incubator-seata/pull/6642)] 修复codecov token找不到导致无法提交单测覆盖度报告
- [[#6661](https://github.com/apache/incubator-seata/pull/6661)] 修复`tableMeta`缓存定时刷新失效问题
- [[#6668](https://github.com/apache/incubator-seata/pull/6668)] 解决namingserver同一个集群下instance添加和删除时的线程安全问题
- [[#6678](https://github.com/apache/incubator-seata/pull/6678)] 修复由于表名大小写问题导致的相同记录生成不同RowKey的问题


### optimize:
Expand Down Expand Up @@ -59,5 +60,6 @@
- [xingfudeshi](https://github.com/xingfudeshi)
- [wuwen5](https://github.com/wuwen5)
- [iAmClever](https://github.com/iAmClever)
- [GoodBoyCoder](https://github.com/GoodBoyCoder)

同时,我们收到了社区反馈的很多有价值的issue和建议,非常感谢大家。
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ public String getTableName() {
@Override
protected TableMeta resultSetMetaToSchema(DatabaseMetaData dbmd, String tableName) throws SQLException {
TableMeta result = new TableMeta();
result.setTableName(tableName);

TableNameMeta tableNameMeta = toTableNameMeta(tableName, dbmd.getConnection().getSchema());
result.setTableName(tableNameMeta.getTableName());
try (ResultSet rsColumns = dbmd.getColumns("", tableNameMeta.getSchema(), tableNameMeta.getTableName(), "%");
ResultSet rsIndex = dbmd.getIndexInfo(null, tableNameMeta.getSchema(), tableNameMeta.getTableName(), false, true);
ResultSet rsPrimary = dbmd.getPrimaryKeys(null, tableNameMeta.getSchema(), tableNameMeta.getTableName())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ protected TableMeta fetchSchema(Connection connection, String tableName) throws

protected TableMeta resultSetMetaToSchema(DatabaseMetaData dbmd, String tableName) throws SQLException {
TableMeta tm = new TableMeta();
tm.setTableName(tableName);
String[] schemaTable = tableName.split("\\.");
String schemaName = schemaTable.length > 1 ? schemaTable[0] : dbmd.getUserName();
tableName = schemaTable.length > 1 ? schemaTable[1] : tableName;
Expand All @@ -91,6 +90,10 @@ protected TableMeta resultSetMetaToSchema(DatabaseMetaData dbmd, String tableNam
} else {
tableName = tableName.toUpperCase();
}
// https://github.com/apache/incubator-seata/issues/6612
// The parsed table name may contain both uppercase and lowercase letters,
// resulting in inconsistent metadata information for the same table on different clients.
tm.setTableName(tableName);
tm.setCaseSensitive(StringUtils.hasLowerCase(tableName));

try (ResultSet rsColumns = dbmd.getColumns("", schemaName, tableName, "%");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ private TableMeta resultSetMetaToSchema(Connection connection, String tableName)

try (ResultSet rsColumns = dbmd.getColumns(null, schemaName, tableName, "%");
ResultSet rsIndex = dbmd.getIndexInfo(null, schemaName, tableName, false, true);
ResultSet rsTable = dbmd.getTables(null, schemaName, tableName, new String[]{"TABLE"});
ResultSet rsPrimary = dbmd.getPrimaryKeys(null, schemaName, tableName)) {
while (rsColumns.next()) {
ColumnMeta col = new ColumnMeta();
Expand Down Expand Up @@ -182,6 +183,19 @@ private TableMeta resultSetMetaToSchema(Connection connection, String tableName)
if (tm.getAllIndexes().isEmpty()) {
throw new ShouldNeverHappenException("Could not found any index in the table: " + tableName);
}

while (rsTable.next()) {
String rsTableName = rsTable.getString("TABLE_NAME");
String rsTableSchema = rsTable.getString("TABLE_SCHEM");
//set origin tableName with schema if necessary
if ("public".equalsIgnoreCase(rsTableSchema)) {
//for compatibility reasons, old clients generally do not have the 'public' default schema by default.
tm.setTableName(rsTableName);
} else {
//without schema, different records with the same primary key value and the same table name in different schemas may have the same lock record.
tm.setTableName(rsTableSchema + "." + rsTableName);
}
}
}

return tm;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ private TableMeta resultSetMetaToSchema(Connection connection, String tableName)
DatabaseMetaData metaData = connection.getMetaData();
try (ResultSet rsColumns = metaData.getColumns(catalogName, schemaName, pureTableName, "%");
ResultSet rsIndex = metaData.getIndexInfo(catalogName, schemaName, pureTableName, false, true);
ResultSet rsTable = metaData.getTables(catalogName, schemaName, pureTableName, new String[]{"TABLE"});
ResultSet rsPrimary = metaData.getPrimaryKeys(catalogName, schemaName, pureTableName)) {
//get column metaData
while (rsColumns.next()) {
Expand Down Expand Up @@ -186,6 +187,19 @@ private TableMeta resultSetMetaToSchema(Connection connection, String tableName)
if (tm.getAllIndexes().isEmpty()) {
throw new ShouldNeverHappenException(String.format("Could not found any index in the table: %s", tableName));
}

while (rsTable.next()) {
String rsTableName = rsTable.getString("TABLE_NAME");
String rsTableSchema = rsTable.getString("TABLE_SCHEM");
//set origin tableName with schema if necessary
if ("dbo".equalsIgnoreCase(rsTableSchema)) {
//for compatibility reasons, old clients generally do not have the 'dbo' default schema by default.
tm.setTableName(rsTableName);
} else {
//without schema, different records with the same primary key value and the same table name in different schemas may have the same lock record.
tm.setTableName(rsTableSchema + "." + rsTableName);
}
}
}
return tm;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ public class MockDatabaseMetaData implements DatabaseMetaData {
"PSEUDO_COLUMN"
);

private static List<String> tableMetaColumnLabels = Arrays.asList(
"TABLE_NAME",
"TABLE_SCHEM"
);

private Object[][] columnsMetasReturnValue;

private Object[][] indexMetasReturnValue;
Expand All @@ -86,6 +91,8 @@ public class MockDatabaseMetaData implements DatabaseMetaData {

private Object[][] mockColumnsMetasReturnValue;

private Object[][] mockTableMetasReturnValue;

/**
* Instantiate a new MockDatabaseMetaData
*/
Expand All @@ -95,6 +102,7 @@ public MockDatabaseMetaData(MockConnection connection) {
this.indexMetasReturnValue = connection.getDriver().getMockIndexMetasReturnValue();
this.pkMetasReturnValue = connection.getDriver().getMockPkMetasReturnValue();
this.mockColumnsMetasReturnValue = connection.getDriver().getMockOnUpdateColumnsReturnValue();
this.mockTableMetasReturnValue = connection.getDriver().getMockTableMetasReturnValue();
}

@Override
Expand Down Expand Up @@ -702,7 +710,8 @@ public ResultSet getProcedureColumns(String catalog, String schemaPattern, Strin
@Override
public ResultSet getTables(String catalog, String schemaPattern, String tableNamePattern, String[] types)
throws SQLException {
return null;
return new MockResultSet(this.connection.createStatement())
.mockResultSet(tableMetaColumnLabels, mockTableMetasReturnValue);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public class MockDriver extends com.alibaba.druid.mock.MockDriver {
*/
private Object[][] mockPkMetasReturnValue;

/**
* the mock value of table meta return value
*/
private Object[][] mockTableMetasReturnValue;

/**
*
*/
Expand All @@ -81,25 +86,34 @@ public MockDriver(Object[][] mockColumnsMetasReturnValue, Object[][] mockIndexMe
this(Lists.newArrayList(), new Object[][]{}, mockColumnsMetasReturnValue, mockIndexMetasReturnValue, mockPkMetasReturnValue);
}

public MockDriver(Object[][] mockColumnsMetasReturnValue, Object[][] mockIndexMetasReturnValue, Object[][] mockPkMetasReturnValue, Object[][] mockTableMetasReturnValue) {
this(Lists.newArrayList(), new Object[][]{}, mockColumnsMetasReturnValue, mockIndexMetasReturnValue, mockPkMetasReturnValue, mockTableMetasReturnValue);
}

public MockDriver(List<String> mockReturnValueColumnLabels, Object[][] mockReturnValue, Object[][] mockColumnsMetasReturnValue, Object[][] mockIndexMetasReturnValue) {
this(mockReturnValueColumnLabels, mockReturnValue, mockColumnsMetasReturnValue, mockIndexMetasReturnValue, new Object[][]{});
}

public MockDriver(List<String> mockReturnValueColumnLabels, Object[][] mockReturnValue, Object[][] mockColumnsMetasReturnValue, Object[][] mockIndexMetasReturnValue, Object[][] mockPkMetasReturnValue) {
this(mockReturnValueColumnLabels, mockReturnValue, mockColumnsMetasReturnValue, mockIndexMetasReturnValue, mockPkMetasReturnValue, new Object[][]{});
this(mockReturnValueColumnLabels, mockReturnValue, mockColumnsMetasReturnValue, mockIndexMetasReturnValue, mockPkMetasReturnValue, new Object[][]{}, new Object[][]{});
}

public MockDriver(List<String> mockReturnValueColumnLabels, Object[][] mockReturnValue, Object[][] mockColumnsMetasReturnValue, Object[][] mockIndexMetasReturnValue, Object[][] mockPkMetasReturnValue, Object[][] mockTableMetasReturnValue) {
this(mockReturnValueColumnLabels, mockReturnValue, mockColumnsMetasReturnValue, mockIndexMetasReturnValue, mockPkMetasReturnValue, new Object[][]{}, mockTableMetasReturnValue);
}

/**
* Instantiate a new MockDriver
*/
public MockDriver(List<String> mockReturnValueColumnLabels, Object[][] mockReturnValue, Object[][] mockColumnsMetasReturnValue, Object[][] mockIndexMetasReturnValue, Object[][] mockPkMetasReturnValue, Object[][] mockOnUpdateColumnsReturnValue) {
public MockDriver(List<String> mockReturnValueColumnLabels, Object[][] mockReturnValue, Object[][] mockColumnsMetasReturnValue, Object[][] mockIndexMetasReturnValue, Object[][] mockPkMetasReturnValue, Object[][] mockOnUpdateColumnsReturnValue, Object[][] mockTableMetasReturnValue) {
this.mockReturnValueColumnLabels = mockReturnValueColumnLabels;
this.mockReturnValue = mockReturnValue;
this.mockColumnsMetasReturnValue = mockColumnsMetasReturnValue;
this.mockIndexMetasReturnValue = mockIndexMetasReturnValue;
this.mockPkMetasReturnValue = mockPkMetasReturnValue;
this.setMockExecuteHandler(new MockExecuteHandlerImpl(mockReturnValueColumnLabels, mockReturnValue, mockColumnsMetasReturnValue));
this.mockOnUpdateColumnsReturnValue = mockOnUpdateColumnsReturnValue;
this.mockTableMetasReturnValue = mockTableMetasReturnValue;
}

/**
Expand Down Expand Up @@ -198,4 +212,12 @@ public Object[][] getMockOnUpdateColumnsReturnValue() {
public void setMockOnUpdateColumnsReturnValue(Object[][] mockOnUpdateColumnsReturnValue) {
this.mockOnUpdateColumnsReturnValue = mockOnUpdateColumnsReturnValue;
}

public Object[][] getMockTableMetasReturnValue() {
return mockTableMetasReturnValue;
}

public void setMockTableMetasReturnValue(Object[][] mockTableMetasReturnValue) {
this.mockTableMetasReturnValue = mockTableMetasReturnValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,14 @@ public class PostgresqlTableMetaCacheTest {
new Object[] {"id"}
};

private static Object[][] tableMetas =
new Object[][]{
new Object[]{"pt1", "public"}
};

@Test
public void getTableMetaTest() throws SQLException {
MockDriver mockDriver = new MockDriver(columnMetas, indexMetas, pkMetas);
MockDriver mockDriver = new MockDriver(columnMetas, indexMetas, pkMetas, tableMetas);
DruidDataSource dataSource = new DruidDataSource();
dataSource.setUrl("jdbc:mock:xxx");
dataSource.setDriver(mockDriver);
Expand All @@ -73,6 +78,9 @@ public void getTableMetaTest() throws SQLException {
TableMeta tableMeta = tableMetaCache.getTableMeta(proxy.getPlainConnection(), "pt1", proxy.getResourceId());

Assertions.assertNotNull(tableMeta);
tableMeta = tableMetaCache.getTableMeta(proxy.getPlainConnection(), "Pt1", proxy.getResourceId());
Assertions.assertNotNull(tableMeta);
Assertions.assertEquals("pt1", tableMeta.getTableName());

tableMeta = tableMetaCache.getTableMeta(proxy.getPlainConnection(), "t.pt1", proxy.getResourceId());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@
public class SqlServerTableMetaCacheTest {
private static Object[][] columnMetas =
new Object[][]{
new Object[]{"", "", "mt1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[]{"", "", "mt1", "name1", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
new Object[]{"", "", "st1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[]{"", "", "st1", "name1", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
"NO"},
new Object[]{"", "", "mt1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
new Object[]{"", "", "st1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
"NO"},
new Object[]{"", "", "mt1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
new Object[]{"", "", "st1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
"NO"}
};
private static Object[][] indexMetas =
Expand All @@ -61,6 +61,11 @@ public class SqlServerTableMetaCacheTest {
new Object[]{"id"}
};

private static Object[][] tableMetas =
new Object[][]{
new Object[]{"st1", "m"}
};

private TableMetaCache getTableMetaCache() {
return TableMetaCacheFactory.getTableMetaCache(JdbcConstants.SQLSERVER);
}
Expand All @@ -79,16 +84,16 @@ public void testTableMeta() {
@Test
public void getTableMetaTest_0() throws SQLException {

MockDriver mockDriver = new MockDriver(columnMetas, indexMetas, pkMetas);
MockDriver mockDriver = new MockDriver(columnMetas, indexMetas, pkMetas, tableMetas);
DruidDataSource dataSource = new DruidDataSource();
dataSource.setUrl("jdbc:mock:xxx");
dataSource.setDriver(mockDriver);

DataSourceProxy proxy = DataSourceProxyTest.getDataSourceProxy(dataSource);

TableMeta tableMeta = getTableMetaCache().getTableMeta(proxy.getPlainConnection(), "m.mt1", proxy.getResourceId());
TableMeta tableMeta = getTableMetaCache().getTableMeta(proxy.getPlainConnection(), "m.st1", proxy.getResourceId());

Assertions.assertEquals("m.mt1", tableMeta.getTableName());
Assertions.assertEquals("m.st1", tableMeta.getTableName());
Assertions.assertEquals("id", tableMeta.getPrimaryKeyOnlyName().get(0));

Assertions.assertEquals("id", tableMeta.getColumnMeta("id").getColumnName());
Expand All @@ -115,12 +120,12 @@ public void getTableMetaTest_0() throws SQLException {
};
mockDriver.setMockIndexMetasReturnValue(indexMetas);
Assertions.assertThrows(ShouldNeverHappenException.class, () -> {
getTableMetaCache().getTableMeta(proxy.getPlainConnection(), "mt2", proxy.getResourceId());
getTableMetaCache().getTableMeta(proxy.getPlainConnection(), "st2", proxy.getResourceId());
});

mockDriver.setMockColumnsMetasReturnValue(null);
Assertions.assertThrows(ShouldNeverHappenException.class, () -> {
getTableMetaCache().getTableMeta(proxy.getPlainConnection(), "mt2", proxy.getResourceId());
getTableMetaCache().getTableMeta(proxy.getPlainConnection(), "st2", proxy.getResourceId());
});

//can not cover the way to get from connection because the mockConnection not support
Expand Down

0 comments on commit 1c0a442

Please sign in to comment.