Fix to only cancel query once (#2152)

Change DefaultOrmQuery.cancel() to call underlying jdbc cancel once

- Refactor tidy already cancelled check (pre query execution)
- Remove unnecessary extra transaction.end() call on future query execution (as already handled by CallableQueryList etc)
This commit is contained in:
Rob Bygrave
2021-02-03 14:51:42 +13:00
committed by GitHub
parent fca54dbbe3
commit 0be98d7989
4 changed files with 19 additions and 45 deletions
@@ -296,10 +296,10 @@ public class CQuery<T> implements DbReadContext, CancelableQuery, SpiProfileTran
this.cancelled = true;
if (pstmt != null) {
try {
logger.debug("Cancelling query");
pstmt.cancel();
} catch (SQLException e) {
String msg = "Error cancelling query";
throw new PersistenceException(msg, e);
throw new PersistenceException("Error cancelling query", e);
}
}
} finally {
@@ -322,7 +322,6 @@ public class CQuery<T> implements DbReadContext, CancelableQuery, SpiProfileTran
}
private boolean prepareBindExecuteQueryWithOption(boolean forwardOnlyHint) throws SQLException {
ResultSet resultSet = prepareResultSet(forwardOnlyHint);
if (resultSet == null) {
return false;
@@ -334,19 +333,12 @@ public class CQuery<T> implements DbReadContext, CancelableQuery, SpiProfileTran
ResultSet prepareResultSet(boolean forwardOnlyHint) throws SQLException {
lock.lock();
try {
if (cancelled || query.isCancelled()) {
// cancelled before we started
cancelled = true;
return null;
if (cancelled) {
throw new SQLException("Query cancelled");
}
startNano = System.nanoTime();
// prepare
SpiTransaction t = request.getTransaction();
profileOffset = t.profileOffset();
Connection conn = t.getInternalConnection();
if (query.isRawSql()) {
ResultSet suppliedResultSet = query.getRawSql().getResultSet();
if (suppliedResultSet != null) {
@@ -356,6 +348,7 @@ public class CQuery<T> implements DbReadContext, CancelableQuery, SpiProfileTran
}
}
Connection conn = t.getInternalConnection();
if (forwardOnlyHint) {
// Use forward only hints for large resultSet processing (Issue 56, MySql specific)
pstmt = conn.prepareStatement(sql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
@@ -363,18 +356,13 @@ public class CQuery<T> implements DbReadContext, CancelableQuery, SpiProfileTran
} else {
pstmt = conn.prepareStatement(sql);
}
if (query.getTimeout() > 0) {
pstmt.setQueryTimeout(query.getTimeout());
}
if (query.getBufferFetchSizeHint() > 0) {
pstmt.setFetchSize(query.getBufferFetchSizeHint());
}
DataBind dataBind = queryPlan.bindEncryptedProperties(pstmt, conn);
bindLog = predicates.bind(dataBind);
// executeQuery
bindLog = predicates.bind(queryPlan.bindEncryptedProperties(pstmt, conn));
return pstmt.executeQuery();
} finally {
lock.unlock();
@@ -401,12 +401,6 @@ public class CQueryEngine {
if (cquery != null) {
cquery.close();
}
if (request.getQuery().isFutureFetch()) {
// end the transaction for futureFindIds
// as it had it's own transaction
logger.debug("Future fetch completed!");
request.getTransaction().end();
}
}
}
@@ -751,7 +751,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
@Override
public NaturalKeyQueryData<T> naturalKey() {
if (whereExpressions == null) {
return null;
}
@@ -767,7 +766,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
return null;
}
}
return data;
}
@@ -815,7 +813,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
copy.m2mIncludeJoin = m2mIncludeJoin;
copy.profilingListener = profilingListener;
copy.profileLocation = profileLocation;
copy.baseTable = baseTable;
copy.rootTableAlias = rootTableAlias;
copy.distinct = distinct;
@@ -1100,7 +1097,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
@Override
public ObjectGraphNode setOrigin(CallOrigin callOrigin) {
// create a 'origin' which links this query to the profiling information
ObjectGraphOrigin o = new ObjectGraphOrigin(calculateOriginQueryHash(), callOrigin, beanType.getName());
parentNode = new ObjectGraphNode(o, null);
@@ -1241,7 +1237,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
*/
@Override
public CQueryPlanKey prepare(SpiOrmQueryRequest<T> request) {
prepareExpressions(request);
prepareForPaging();
queryPlanKey = createQueryPlanKey();
@@ -1252,7 +1247,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
* Prepare the expressions (compile sub-queries etc).
*/
private void prepareExpressions(BeanQueryRequest<?> request) {
if (whereExpressions != null) {
whereExpressions.prepareExpression(request);
}
@@ -1267,7 +1261,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
* case, this is not a distinct query
*/
private void prepareForPaging() {
// add the rawSql statement - if any
if (orderByIsEmpty()) {
if (rawSql != null && rawSql.getSql() != null) {
@@ -1678,7 +1671,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
return this;
}
}
if (bindParams == null) {
bindParams = new BindParams();
}
@@ -1972,7 +1964,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
if (namedParams == null) {
namedParams = new HashMap<>();
}
return namedParams.computeIfAbsent(name, ONamedParam::new);
}
@@ -2066,8 +2057,8 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
public void cancel() {
lock.lock();
try {
cancelled = true;
if (cancelableQuery != null) {
if (!cancelled && cancelableQuery != null) {
cancelled = true;
cancelableQuery.cancel();
}
} finally {
@@ -2095,7 +2086,6 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
*/
@Override
public Set<String> validate(BeanType<T> desc) {
SpiExpressionValidation validation = new SpiExpressionValidation(desc);
if (whereExpressions != null) {
whereExpressions.validate(validation);