Copilot commented on code in PR #6348:
URL: https://github.com/apache/incubator-seata/pull/6348#discussion_r2190292502
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/exec/UpdateExecutor.java:
##########
@@ -97,20 +98,30 @@ protected String buildBeforeImageSQL(TableMeta tableMeta,
ArrayList<List<Object>
@Override
protected TableRecords afterImage(TableRecords beforeImage) throws
SQLException {
- TableMeta tmeta = getTableMeta();
if (beforeImage == null || beforeImage.size() == 0) {
return TableRecords.empty(getTableMeta());
}
- String selectSQL = buildAfterImageSQL(tmeta, beforeImage);
+ TableMeta tmeta = getTableMeta();
PreparedStatement pst = null;
ResultSet rs = null;
- try {
- pst = statementProxy.getConnection().prepareStatement(selectSQL);
- SqlGenerateUtils.setParamForPk(beforeImage.pkRows(),
getTableMeta().getPrimaryKeyOnlyName(), pst);
- rs = pst.executeQuery();
- return TableRecords.buildRecords(tmeta, rs);
- } finally {
- IOUtil.close(rs, pst);
+ SQLUpdateRecognizer recognizer = (SQLUpdateRecognizer) sqlRecognizer;
+ List<String> whereColumns = recognizer.getWhereColumns();
+ boolean contain =
+ statementProxy.getConnection().getTransactionIsolation() ==
Connection.TRANSACTION_REPEATABLE_READ
+ || tmeta.containsPK(whereColumns);
+ if (contain) {
+ String selectSQL = buildAfterImageSQL(tmeta, beforeImage);
+ try {
+ pst =
statementProxy.getConnection().prepareStatement(selectSQL);
+ SqlGenerateUtils.setParamForPk(
+ beforeImage.pkRows(),
getTableMeta().getPrimaryKeyOnlyName(), pst);
+ rs = pst.executeQuery();
+ return TableRecords.buildRecords(tmeta, rs);
+ } finally {
+ IOUtil.close(rs, pst);
+ }
+ } else {
+ return beforeImage();
Review Comment:
This attempts to call `beforeImage()` but `beforeImage` is the method
parameter (a `TableRecords`), causing a compile error. It should return the
parameter directly: `return beforeImage;`
```suggestion
return beforeImage;
```
##########
sqlparser/seata-sqlparser-druid/src/main/java/org/apache/seata/sqlparser/druid/sqlserver/SqlServerUpdateRecognizer.java:
##########
@@ -202,6 +202,12 @@ public String getOrderByCondition(ParametersHolder
parametersHolder, ArrayList<L
return null;
}
+ @Override
+ public List<String> getWhereColumns() {
Review Comment:
[nitpick] The `getWhereColumns` override is duplicated across every
dialect-specific recognizer. Consider moving this logic into a shared abstract
base class or default method to reduce code duplication.
##########
sqlparser/seata-sqlparser-druid/src/main/java/org/apache/seata/sqlparser/druid/sqlserver/SqlServerUpdateRecognizer.java:
##########
@@ -202,6 +202,12 @@ public String getOrderByCondition(ParametersHolder
parametersHolder, ArrayList<L
return null;
}
+ @Override
+ public List<String> getWhereColumns() {
+ SQLExpr where = ast.getWhere();
+ return ColumnUtils.delEscape(super.getWhereColumns(where),
getDbType());
Review Comment:
If `ast.getWhere()` is null (no WHERE clause), this will NPE. Add a null
check: e.g. `if (where == null) return Collections.emptyList();` before calling
`super.getWhereColumns(where)`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
