You keep creating new prepared statements and assigning them to ps
, but you only ever close the last one. If you're needing to execute a sequence of statements, either close them as you go or put them in separate variables so you can close them all at the end.
Unclosed statement driving me crazy
-
21-07-2023 - |
Pregunta
I have this code:
Helper helper = getDBhelper();
Connection conn = helper.getConnection();
String sql =
" SELECT DISTINCT date as plannedDate" +
" FROM plan ";
String AssimilationTabDates =
" SELECT DISTINCT payment as plannedDate " +
" FROM credit_mo_assimilation_tab ";
String assimilationInfoDates =
" SELECT DISTINCT planned_pay as plannedDate " +
" FROM assimilation ";
String sqlUpdate =
" UPDATE plan " +
" SET date = ? " +
" WHERE planned_date = ? ";
String updateAssimilationTab =
" UPDATE assimilation " +
" SET payment = to_date(?, 'DD-MM-YYYY') " +
" WHERE d_payment = to_date(?, 'DD-MM-YYYY') ";
String updateAssimilationInfo =
" UPDATE assimilation " +
" SET pay = to_date(?, 'DD-MM-YYYY') " +
" WHERE planned_pay = ? ";
PreparedStatement ps = null;
ResultSet rs = null;
Date plannedDate = null;
List<String> plannedDates = new ArrayList<String>();
List<String> plannedTabDates = new ArrayList<String>();
List<String> plannedInfoDates = new ArrayList<String>();
try {
ps = conn.prepareStatement(sql);
ps.execute();
rs = ps.getResultSet();
while(rs.next()) {
plannedDate = rs.getDate("plannedDate");
String plannedD = new SimpleDateFormat("dd-MMM-yy").format(plannedDate);
plannedDates.add(plannedD);
}
ps = conn.prepareStatement(AssimilationTabDates);
ps.execute();
rs = ps.getResultSet();
while(rs.next()) {
plannedDate = rs.getDate("plannedDate");
String plannedD = null;
if(plannedDate != null){
plannedD = new SimpleDateFormat("dd-MMM-yy").format(plannedDate);
}
if(plannedD != null){
plannedTabDates.add(plannedD);
}
}
ps = conn.prepareStatement(assimilationInfoDates);
ps.execute();
rs = ps.getResultSet();
while(rs.next()) {
plannedDate = rs.getDate("plannedDate");
String plannedD = new SimpleDateFormat("dd-MMM-yy").format(plannedDate);
plannedInfoDates.add(plannedD);
}
} catch (Exception ex) {
ex.printStackTrace();
}
try{
for( String oneItem : plannedDates ) {
SimpleDateFormat formatter = new SimpleDateFormat("dd-MMM-yy");
Date datePlanned = formatter.parse(oneItem);
Date paymentDate = datePlanned;
Date paymentDateReal = paymentDate;
if (paymentDate != null) {
paymentDateReal = DefaultProdCalendar.findNearestWorkingDay(paymentDate);
}
String realDate = new SimpleDateFormat("dd-MMM-yy").format(paymentDateReal);
ps = conn.prepareStatement(sqlUpdate);
ps.setString(1, realDate);
ps.setString(2, oneItem);
ps.execute();
rs = ps.getResultSet();
}
for( String oneItem : plannedTabDates ) {
SimpleDateFormat formatter = new SimpleDateFormat("dd-MMM-yy");
Date datePlanned = formatter.parse(oneItem);
Date paymentDate = datePlanned;
Date paymentDateReal = paymentDate;
if (paymentDate != null) {
paymentDateReal = DefaultProdCalendar.findNearestWorkingDay(paymentDate);
}
String realDate = new SimpleDateFormat("dd-MMM-yy").format(paymentDateReal);
ps = conn.prepareStatement(updateAssimilationTab);
ps.setString(1, realDate);
ps.setString(2, oneItem);
ps.execute();
rs = ps.getResultSet();
}
for( String oneItem : plannedInfoDates ) {
SimpleDateFormat formatter = new SimpleDateFormat("dd-MMM-yy");
Date datePlanned = formatter.parse(oneItem);
Date paymentDate = datePlanned;
Date paymentDateReal = paymentDate;
if (paymentDate != null) {
paymentDateReal = DefaultProdCalendar.findNearestWorkingDay(paymentDate);
}
String realDate = new SimpleDateFormat("dd-MMM-yy").format(paymentDateReal);
ps = conn.prepareStatement(updateAssimilationInfo);
ps.setString(1, realDate);
ps.setString(2, oneItem);
ps.execute();
rs = ps.getResultSet();
}
}catch(Exception e){
e.printStackTrace();
}
try {
ps = conn.prepareStatement(sql);
ps.execute();
rs = ps.getResultSet();
while(rs.next()) {
plannedDate = rs.getDate("plannedDate");
String plannedD = new SimpleDateFormat("dd-MMM-yy").format(plannedDate);
plannedDates.add(plannedD);
}
ps = conn.prepareStatement(AssimilationTabDates);
ps.execute();
rs = ps.getResultSet();
while(rs.next()) {
plannedDate = rs.getDate("plannedDate");
String plannedD = null;
if(plannedDate != null){
plannedD = new SimpleDateFormat("dd-MMM-yy").format(plannedDate);
}
if(plannedD != null){
plannedTabDates.add(plannedD);
}
}
} catch (Exception ex) {
ex.printStackTrace();
}
try{
for( String oneItem : plannedDates ) {
SimpleDateFormat formatter = new SimpleDateFormat("dd-MMM-yy");
Date datePlanned = formatter.parse(oneItem);
Date paymentDate = datePlanned;
Date paymentDateReal = paymentDate;
if (paymentDate != null) {
paymentDateReal = DefaultProdCalendar.findNearestWorkingDay(paymentDate);
}
String realDate = new SimpleDateFormat("dd-MMM-yy").format(paymentDateReal);
ps = conn.prepareStatement(sqlUpdate);
ps.setString(1, realDate);
ps.setString(2, oneItem);
ps.execute();
rs = ps.getResultSet();
}
for( String oneItem : plannedTabDates ) {
SimpleDateFormat formatter = new SimpleDateFormat("dd-MMM-yy");
Date datePlanned = formatter.parse(oneItem);
Date paymentDate = datePlanned;
Date paymentDateReal = paymentDate;
if (paymentDate != null) {
paymentDateReal = DefaultProdCalendar.findNearestWorkingDay(paymentDate);
}
String realDate = new SimpleDateFormat("dd-MMM-yy").format(paymentDateReal);
ps = conn.prepareStatement(updateAssimilationTab);
ps.setString(1, realDate);
ps.setString(2, oneItem);
ps.execute();
rs = ps.getResultSet();
}
}catch(Exception e){
e.printStackTrace();
}
finally {
try {
ps.close();
conn.close();
} catch (SQLException e) {
e.printStackTrace();
}
}
The above code runs perfectly fine, but there is a small issue. The statement is not getting closed, so this is pushing my jboss
server to do it and it throws the following error for almost all the query executions:
Closing a statement you left open, please do your own housekeeping: java.lang.Throwable: STACKTRACE
Aren't
ps.close();
conn.close();
In the finally
supposed to close everything up?
I know that I'm missing something extremely small and simple, but I'm not able to spot it right now.
Solución
Otros consejos
If we agree that a method should do one thing well, I'd say this is a bad implementation.
Each of those SQL operations ought to have a separate method. Each try/catch/finally
should create and close resources in the narrowest scope possible (method scope).
You can't have commit/rollback logic if you acquire the Connection
. I'd suggest passing it into your SQL methods. Let a service or business layer, that knows about units of work, acquire the Connection
and commit or rollback.
I'd also say you're repeating a lot of code unnecessarily. I'd refactor to make this DRY-er.
In addition to what chrylis said, your exception handling is flawed. When you catch an exception, it won't propagate to the enclosing try/catch
. So you should replace your catch-clauses with this:
} catch (Exception ex) {
ex.printStackTrace(); // I'd also replace this by something useful (ie. logging)
} finally {
ps.close();
}