Copilot commented on code in PR #7456:
URL: https://github.com/apache/incubator-seata/pull/7456#discussion_r2158709688
##########
sqlparser/seata-sqlparser-druid/src/main/java/org/apache/seata/sqlparser/druid/DruidSQLRecognizerFactoryImpl.java:
##########
@@ -38,43 +38,65 @@
class DruidSQLRecognizerFactoryImpl implements SQLRecognizerFactory {
@Override
public List<SQLRecognizer> create(String sql, String dbType) {
- List<SQLStatement> asts = SQLUtils.parseStatements(sql,
DruidDbTypeAdapter.getAdaptiveDbType(dbType));
- if (CollectionUtils.isEmpty(asts)) {
- throw new UnsupportedOperationException("Unsupported SQL: " + sql);
- }
- if (asts.size() > 1 && !(asts.stream().allMatch(statement -> statement
instanceof SQLUpdateStatement)
- || asts.stream().allMatch(statement -> statement instanceof
SQLDeleteStatement))) {
- throw new UnsupportedOperationException("ONLY SUPPORT SAME TYPE
(UPDATE OR DELETE) MULTI SQL -" + sql);
- }
List<SQLRecognizer> recognizers = null;
Review Comment:
The `recognizers` list is declared but never initialized, causing a
NullPointerException when adding elements. Initialize it (e.g., `recognizers =
new ArrayList<>()`) before the loop.
```suggestion
List<SQLRecognizer> recognizers = new ArrayList<>();
```
##########
sqlparser/seata-sqlparser-druid/src/main/java/org/apache/seata/sqlparser/druid/DruidSQLRecognizerFactoryImpl.java:
##########
@@ -38,43 +38,65 @@
class DruidSQLRecognizerFactoryImpl implements SQLRecognizerFactory {
@Override
public List<SQLRecognizer> create(String sql, String dbType) {
- List<SQLStatement> asts = SQLUtils.parseStatements(sql,
DruidDbTypeAdapter.getAdaptiveDbType(dbType));
- if (CollectionUtils.isEmpty(asts)) {
- throw new UnsupportedOperationException("Unsupported SQL: " + sql);
- }
- if (asts.size() > 1 && !(asts.stream().allMatch(statement -> statement
instanceof SQLUpdateStatement)
- || asts.stream().allMatch(statement -> statement instanceof
SQLDeleteStatement))) {
- throw new UnsupportedOperationException("ONLY SUPPORT SAME TYPE
(UPDATE OR DELETE) MULTI SQL -" + sql);
- }
List<SQLRecognizer> recognizers = null;
SQLRecognizer recognizer = null;
- for (SQLStatement ast : asts) {
- SQLOperateRecognizerHolder recognizerHolder =
-
SQLOperateRecognizerHolderFactory.getSQLRecognizerHolder(dbType.toLowerCase());
- if (ast instanceof SQLInsertStatement) {
- recognizer = recognizerHolder.getInsertRecognizer(sql, ast);
- } else if (ast instanceof SQLUpdateStatement) {
- recognizer = recognizerHolder.getUpdateRecognizer(sql, ast);
- } else if (ast instanceof SQLDeleteStatement) {
- recognizer = recognizerHolder.getDeleteRecognizer(sql, ast);
- } else if (ast instanceof SQLSelectStatement) {
- recognizer =
recognizerHolder.getSelectForUpdateRecognizer(sql, ast);
+ try {
+ List<SQLStatement> asts = SQLUtils.parseStatements(sql,
DruidDbTypeAdapter.getAdaptiveDbType(dbType));
+ if (CollectionUtils.isEmpty(asts)) {
+ throw new UnsupportedOperationException("Unsupported SQL: " +
sql);
}
-
- // When recognizer is null, it indicates that recognizerHolder
cannot allocate unsupported syntax, like merge and replace
- if (ast instanceof SQLReplaceStatement) {
- //just like:replace into t (id,dr) values (1,'2'), (2,'3')
- throw new NotSupportYetException("not support the sql syntax
with ReplaceStatement:" + ast +
- "\nplease see the doc about SQL restrictions
https://seata.apache.org/zh-cn/docs/user/sqlreference/dml";);
+ if (asts.size() > 1 && !(asts.stream().allMatch(statement ->
statement instanceof SQLUpdateStatement)
+ || asts.stream().allMatch(statement -> statement
instanceof SQLDeleteStatement))) {
+ throw new UnsupportedOperationException("ONLY SUPPORT SAME
TYPE (UPDATE OR DELETE) MULTI SQL -" + sql);
}
+ for (SQLStatement ast : asts) {
+ SQLOperateRecognizerHolder recognizerHolder =
+
SQLOperateRecognizerHolderFactory.getSQLRecognizerHolder(dbType.toLowerCase());
+ if (ast instanceof SQLInsertStatement) {
+ recognizer = recognizerHolder.getInsertRecognizer(sql,
ast);
+ } else if (ast instanceof SQLUpdateStatement) {
+ recognizer = recognizerHolder.getUpdateRecognizer(sql,
ast);
+ } else if (ast instanceof SQLDeleteStatement) {
+ recognizer = recognizerHolder.getDeleteRecognizer(sql,
ast);
+ } else if (ast instanceof SQLSelectStatement) {
+ recognizer =
recognizerHolder.getSelectForUpdateRecognizer(sql, ast);
+ }
- if (recognizer != null && recognizer.isSqlSyntaxSupports()) {
- if (recognizers == null) {
- recognizers = new ArrayList<>();
+ // When recognizer is null, it indicates that recognizerHolder
cannot allocate unsupported syntax, like merge and replace
+ if (ast instanceof SQLReplaceStatement) {
+ //just like:replace into t (id,dr) values (1,'2'), (2,'3')
+ throw new NotSupportYetException("not support the sql
syntax with ReplaceStatement:" + ast +
+ "\nplease see the doc about SQL restrictions
https://seata.apache.org/zh-cn/docs/user/sqlreference/dml";);
}
- recognizers.add(recognizer);
+
+ if (recognizer != null && recognizer.isSqlSyntaxSupports()) {
+ if (recognizers == null) {
+ recognizers = new ArrayList<>();
+ }
+ recognizers.add(recognizer);
+ }
+ }
+ } catch (RuntimeException e) {
+ // Check if it is ParserException by class name, avoid direct
reference
+ if (isParserException(e)) {
+ throw new NotSupportYetException("not support the sql syntax:
" + sql +
+ "\nplease see the doc about SQL restrictions
https://seata.apache.org/zh-cn/docs/user/sqlreference/dml";, e);
}
+ // Other RuntimeExceptions continue to be thrown without
conversion,Ensure that other original test classes can run normally
+ throw e;
}
return recognizers;
}
+
+ /**
+ * Check if the exception is a Druid ParserException
+ * Use class name comparison to avoid directly referencing the
ParserException class
+ */
+ private boolean isParserException(Throwable e) {
+ if (e == null) {
+ return false;
+ }
+ String className = e.getClass().getName();
+ return
"com.alibaba.druid.sql.parser.ParserException".equals(className);
Review Comment:
Enhance `isParserException` by traversing the exception cause chain (e.g.,
`while (e != null) { ...; e = e.getCause(); }`) so wrapped `ParserException`s
are also detected.
```suggestion
while (e != null) {
String className = e.getClass().getName();
if
("com.alibaba.druid.sql.parser.ParserException".equals(className)) {
return true;
}
e = e.getCause();
}
return false;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@seata.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@seata.apache.org
For additional commands, e-mail: notifications-h...@seata.apache.org
