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: ColumnUtils delEscape with scheme error. #2875

Merged
merged 5 commits into from
Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
*/
public final class ColumnUtils {

private static final String DOT = ".";

/**
* The escape
*/
Expand Down Expand Up @@ -107,6 +109,11 @@ public static String delEscape(String colName, Escape escape) {
return colName;
}
if (colName.charAt(0) == escape.value && colName.charAt(colName.length() - 1) == escape.value) {
final String withScheme = escape.value + DOT + escape.value;
int index = colName.indexOf(withScheme);
if (index > -1) {
return colName.substring(1, index) + DOT + colName.substring(index + withScheme.length(), colName.length() - 1);
}
return colName.substring(1, colName.length() - 1);
}
return colName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@
import java.util.List;
import java.util.StringJoiner;

import io.seata.rm.datasource.ColumnUtils;
import io.seata.rm.datasource.StatementProxy;
import io.seata.rm.datasource.sql.struct.TableMeta;
import io.seata.rm.datasource.sql.struct.TableRecords;
import io.seata.rm.datasource.undo.KeywordChecker;
import io.seata.rm.datasource.undo.KeywordCheckerFactory;
import io.seata.sqlparser.SQLDeleteRecognizer;
import io.seata.sqlparser.SQLRecognizer;
import org.apache.commons.lang.StringUtils;
Expand Down Expand Up @@ -62,7 +61,6 @@ protected TableRecords beforeImage() throws SQLException {
}

private String buildBeforeImageSQL(SQLDeleteRecognizer visitor, TableMeta tableMeta, ArrayList<List<Object>> paramAppenderList) {
KeywordChecker keywordChecker = KeywordCheckerFactory.getKeywordChecker(getDbType());
String whereCondition = buildWhereCondition(visitor, paramAppenderList);
StringBuilder suffix = new StringBuilder(" FROM ").append(getFromTableInSQL());
if (StringUtils.isNotBlank(whereCondition)) {
Expand All @@ -71,7 +69,7 @@ private String buildBeforeImageSQL(SQLDeleteRecognizer visitor, TableMeta tableM
suffix.append(" FOR UPDATE");
StringJoiner selectSQLAppender = new StringJoiner(", ", "SELECT ", suffix.toString());
for (String column : tableMeta.getAllColumns().keySet()) {
selectSQLAppender.add(getColumnNameInSQL(keywordChecker.checkAndReplace(column)));
selectSQLAppender.add(getColumnNameInSQL(ColumnUtils.addEscape(column, getDbType())));
}
return selectSQLAppender.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
import io.seata.common.util.StringUtils;


import io.seata.rm.datasource.ColumnUtils;
import io.seata.rm.datasource.StatementProxy;
import io.seata.rm.datasource.sql.struct.TableMeta;
import io.seata.rm.datasource.sql.struct.TableRecords;
import io.seata.rm.datasource.undo.KeywordChecker;
import io.seata.rm.datasource.undo.KeywordCheckerFactory;
import io.seata.sqlparser.SQLDeleteRecognizer;
import io.seata.sqlparser.SQLRecognizer;

Expand Down Expand Up @@ -50,7 +49,6 @@ protected TableRecords beforeImage() throws SQLException {
DeleteExecutor executor = new DeleteExecutor(statementProxy, statementCallback, sqlRecognizers.get(0));
return executor.beforeImage();
}
final KeywordChecker keywordChecker = KeywordCheckerFactory.getKeywordChecker(getDbType());
final TableMeta tmeta = getTableMeta(sqlRecognizers.get(0).getTableName());
final ArrayList<List<Object>> paramAppenderList = new ArrayList<>();
StringBuilder whereCondition = new StringBuilder();
Expand All @@ -75,7 +73,7 @@ protected TableRecords beforeImage() throws SQLException {
suffix.append(" FOR UPDATE");
final StringJoiner selectSQLAppender = new StringJoiner(", ", "SELECT ", suffix.toString());
for (String column : tmeta.getAllColumns().keySet()) {
selectSQLAppender.add(getColumnNameInSQL(keywordChecker.checkAndReplace(column)));
selectSQLAppender.add(getColumnNameInSQL(ColumnUtils.addEscape(column, getDbType())));
}
return buildTableRecords(tmeta, selectSQLAppender.toString(), paramAppenderList);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@

import io.seata.common.exception.ShouldNeverHappenException;
import io.seata.common.loader.LoadLevel;
import io.seata.rm.datasource.ColumnUtils;
import io.seata.rm.datasource.sql.struct.ColumnMeta;
import io.seata.rm.datasource.sql.struct.IndexMeta;
import io.seata.rm.datasource.sql.struct.IndexType;
import io.seata.rm.datasource.sql.struct.TableMeta;
import io.seata.rm.datasource.undo.KeywordChecker;
import io.seata.rm.datasource.undo.KeywordCheckerFactory;
import io.seata.sqlparser.util.JdbcConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -44,8 +43,6 @@ public class MysqlTableMetaCache extends AbstractTableMetaCache {

private static final Logger LOGGER = LoggerFactory.getLogger(MysqlTableMetaCache.class);

private KeywordChecker keywordChecker = KeywordCheckerFactory.getKeywordChecker(JdbcConstants.MYSQL);

@Override
protected String getCacheKey(Connection connection, String tableName, String resourceId) {
StringBuilder cacheKey = new StringBuilder(resourceId);
Expand Down Expand Up @@ -79,7 +76,7 @@ protected String getCacheKey(Connection connection, String tableName, String res

@Override
protected TableMeta fetchSchema(Connection connection, String tableName) throws SQLException {
String sql = "SELECT * FROM " + keywordChecker.checkAndReplace(tableName) + " LIMIT 1";
String sql = "SELECT * FROM " + ColumnUtils.addEscape(tableName, JdbcConstants.MYSQL) + " LIMIT 1";
try (Statement stmt = connection.createStatement();
ResultSet rs = stmt.executeQuery(sql)) {
return resultSetMetaToSchema(rs.getMetaData(), connection.getMetaData());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@
import io.seata.common.exception.ShouldNeverHappenException;
import io.seata.common.loader.LoadLevel;
import io.seata.common.util.StringUtils;
import io.seata.rm.datasource.ColumnUtils;
import io.seata.rm.datasource.sql.struct.ColumnMeta;
import io.seata.rm.datasource.sql.struct.IndexMeta;
import io.seata.rm.datasource.sql.struct.IndexType;
import io.seata.rm.datasource.sql.struct.TableMeta;
import io.seata.rm.datasource.undo.KeywordChecker;
import io.seata.rm.datasource.undo.KeywordCheckerFactory;
import io.seata.sqlparser.util.JdbcConstants;

/**
Expand All @@ -38,8 +37,6 @@
@LoadLevel(name = JdbcConstants.POSTGRESQL)
public class PostgresqlTableMetaCache extends AbstractTableMetaCache {

private KeywordChecker keywordChecker = KeywordCheckerFactory.getKeywordChecker(JdbcConstants.POSTGRESQL);

@Override
protected String getCacheKey(Connection connection, String tableName, String resourceId) {
StringBuilder cacheKey = new StringBuilder(resourceId);
Expand All @@ -64,7 +61,7 @@ protected String getCacheKey(Connection connection, String tableName, String res
protected TableMeta fetchSchema(Connection connection, String tableName) throws SQLException {
try {
DatabaseMetaData dbmd = connection.getMetaData();
tableName = keywordChecker.checkAndReplace(tableName);
tableName = ColumnUtils.addEscape(tableName, JdbcConstants.POSTGRESQL);
return resultSetMetaToSchema(dbmd, tableName);
} catch (SQLException sqlEx) {
throw sqlEx;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,4 @@ public interface KeywordChecker {
*/
boolean checkEscape(String fieldOrTableName);

/**
* check whether given field name and table name use keywords and,if so,will add "`" to the name.
*
* @param fieldOrTableName the field or table name
* @return string
*/
String checkAndReplace(String fieldOrTableName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -1117,9 +1117,4 @@ public boolean checkEscape(String fieldOrTableName) {
return check(fieldOrTableName);
}

@Override
public String checkAndReplace(String fieldOrTableName) {
return check(fieldOrTableName) ? "`" + fieldOrTableName + "`" : fieldOrTableName;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -511,12 +511,6 @@ public boolean checkEscape(String fieldOrTableName) {
return true;
}

@Override
public String checkAndReplace(String fieldOrTableName) {
return check(fieldOrTableName) ? fieldOrTableName : fieldOrTableName;
// return check(fieldOrTableName)?"`" + fieldOrTableName + "`":fieldOrTableName;
}

private static boolean isUppercase(String fieldOrTableName) {
if (fieldOrTableName == null) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,18 +380,6 @@ public boolean checkEscape(String fieldOrTableName) {
return true;
}

@Override
public String checkAndReplace(String fieldOrTableName) {
return check(fieldOrTableName) ? replace(fieldOrTableName) : fieldOrTableName;
}

private String replace(String fieldOrTableName) {
StringBuilder builder = new StringBuilder();
builder.append("\"").append(fieldOrTableName).append("\"");
String name = builder.toString();
return name;
}

private static boolean containsUppercase(String colName) {
if (colName == null) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,21 @@ public void test_delEscape_byEscape() throws Exception {
Assertions.assertEquals("id", cols.get(0));
Assertions.assertEquals("name", cols.get(1));

List<String> cols4 = new ArrayList<>();
cols4.add("`scheme`.`id`");
cols4 = ColumnUtils.delEscape(cols4, ColumnUtils.Escape.MYSQL);
Assertions.assertEquals("scheme.id", cols4.get(0));

List<String> cols2 = new ArrayList<>();
cols2.add("\"id\"");
cols2 = ColumnUtils.delEscape(cols2, ColumnUtils.Escape.STANDARD);
Assertions.assertEquals("id", cols2.get(0));

List<String> cols3 = new ArrayList<>();
cols3.add("\"scheme\".\"id\"");
cols3 = ColumnUtils.delEscape(cols3, ColumnUtils.Escape.STANDARD);
Assertions.assertEquals("scheme.id", cols3.get(0));

Assertions.assertNull(ColumnUtils.delEscape((String) null, ColumnUtils.Escape.MYSQL));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ public class MysqlTableMetaCacheTest {

private static Object[][] columnMetas =
new Object[][] {
new Object[] {"", "", "t1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[] {"", "", "t1", "name1", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
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",
"NO"},
new Object[] {"", "", "t1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
new Object[] {"", "", "mt1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
"NO"},
new Object[] {"", "", "t1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
new Object[] {"", "", "mt1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
"NO"}
};

Expand Down Expand Up @@ -84,9 +84,9 @@ public void getTableMetaTest_0() throws SQLException {

DataSourceProxy proxy = new DataSourceProxy(dataSource);

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

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

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

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

}
Expand All @@ -138,12 +138,12 @@ public void refreshTest_0() throws SQLException {
//change the columns meta
columnMetas =
new Object[][] {
new Object[] {"", "", "t1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[] {"", "", "t1", "name1", Types.VARCHAR, "VARCHAR", 65, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
new Object[] {"", "", "mt1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[] {"", "", "mt1", "name1", Types.VARCHAR, "VARCHAR", 65, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
"NO"},
new Object[] {"", "", "t1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
new Object[] {"", "", "mt1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
"NO"},
new Object[] {"", "", "t1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
new Object[] {"", "", "mt1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
"NO"}
};
mockDriver.setMockColumnsMetasReturnValue(columnMetas);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ public class OracleTableMetaCacheTest {

private static Object[][] columnMetas =
new Object[][] {
new Object[] {"", "", "t1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[] {"", "", "t1", "name1", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
new Object[] {"", "", "ot1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[] {"", "", "ot1", "name1", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
"NO"},
new Object[] {"", "", "t1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
new Object[] {"", "", "ot1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
"NO"},
new Object[] {"", "", "t1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
new Object[] {"", "", "ot1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
"NO"}
};

Expand All @@ -68,11 +68,11 @@ public void getTableMetaTest() throws SQLException {

TableMetaCache tableMetaCache = TableMetaCacheFactory.getTableMetaCache(JdbcConstants.ORACLE);

TableMeta tableMeta = tableMetaCache.getTableMeta(proxy.getPlainConnection(), "t.t1", proxy.getResourceId());
TableMeta tableMeta = tableMetaCache.getTableMeta(proxy.getPlainConnection(), "t.ot1", proxy.getResourceId());

Assertions.assertNotNull(tableMeta);

tableMeta = tableMetaCache.getTableMeta(proxy.getPlainConnection(), "t.\"t1\"", proxy.getResourceId());
tableMeta = tableMetaCache.getTableMeta(proxy.getPlainConnection(), "t.\"ot1\"", proxy.getResourceId());

Assertions.assertNotNull(tableMeta);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ public class PostgresqlTableMetaCacheTest {

private static Object[][] columnMetas =
new Object[][] {
new Object[] {"", "", "t1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[] {"", "", "t1", "name1", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
new Object[] {"", "", "pt1", "id", Types.INTEGER, "INTEGER", 64, 0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"},
new Object[] {"", "", "pt1", "name1", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 2, "YES",
"NO"},
new Object[] {"", "", "t1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
new Object[] {"", "", "pt1", "name2", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 3, "YES",
"NO"},
new Object[] {"", "", "t1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
new Object[] {"", "", "pt1", "name3", Types.VARCHAR, "VARCHAR", 64, 0, 10, 0, "", "", 0, 0, 64, 4, "YES",
"NO"}
};

Expand All @@ -68,15 +68,15 @@ public void getTableMetaTest() throws SQLException {

TableMetaCache tableMetaCache = TableMetaCacheFactory.getTableMetaCache(JdbcConstants.POSTGRESQL);

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

Assertions.assertNotNull(tableMeta);

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

Assertions.assertNotNull(tableMeta);

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

Assertions.assertNotNull(tableMeta);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,6 @@ public void testCheck() {
Assertions.assertTrue(keywordChecker.check("desc"));
}

/**
* Test check and replace
*/
@Test
public void testCheckAndReplace() {
KeywordChecker keywordChecker = KeywordCheckerFactory.getKeywordChecker(JdbcConstants.MYSQL);
Assertions.assertEquals("`desc`", keywordChecker.checkAndReplace("desc"));

}

/**
* Test keyword check with UPDATE case
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,4 @@ public void testOracleKeywordChecker() {
Assertions.assertNotNull(keywordChecker);
}

@Test
public void testCheckAndReplate() {
KeywordChecker keywordChecker = KeywordCheckerFactory.getKeywordChecker(JdbcConstants.ORACLE);
Assertions.assertNull(keywordChecker.checkAndReplace(null));
Assertions.assertEquals("undo_log", keywordChecker.checkAndReplace("undo_log"));
Assertions.assertEquals("TABLE", keywordChecker.checkAndReplace("TABLE"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,4 @@ public void testOracleKeywordChecker() {
Assertions.assertNotNull(keywordChecker);
}

@Test
public void testCheckAndReplate() {
KeywordChecker keywordChecker = KeywordCheckerFactory.getKeywordChecker(JdbcConstants.POSTGRESQL);
Assertions.assertEquals(null, keywordChecker.checkAndReplace(null));
Assertions.assertEquals("undo_log", keywordChecker.checkAndReplace("undo_log"));
Assertions.assertEquals("\"TABLE\"", keywordChecker.checkAndReplace("TABLE"));
}
}