Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #37106 > unrolled thread

Not closing a SQL(ite3) connection

Back to article view | Back to comp.lang.java.programmer

Contents

  Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-09 11:28 +0200
    Re: Not closing a SQL(ite3) connection Arne Vajhøj <arne@vajhoej.dk> - 2017-08-09 18:00 -0400
      Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-11 10:26 +0200
        Re: Not closing a SQL(ite3) connection Arne Vajhøj <arne@vajhoej.dk> - 2017-08-11 20:27 -0400
          Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-12 10:00 +0200
            Re: Not closing a SQL(ite3) connection Andreas Leitgeb <avl@logic.at> - 2017-08-12 08:51 +0000
              Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-12 11:05 +0200
            Re: Not closing a SQL(ite3) connection Arne Vajhøj <arne@vajhoej.dk> - 2017-08-12 09:52 -0400
              Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-12 16:26 +0200
                Re: Not closing a SQL(ite3) connection Arne Vajhøj <arne@vajhoej.dk> - 2017-08-12 21:43 -0400
                  Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-15 08:19 +0200
            Re: Not closing a SQL(ite3) connection lewbloch@gmail.com - 2017-08-12 10:14 -0700
          Re: Not closing a SQL(ite3) connection Andreas Leitgeb <avl@logic.at> - 2017-08-12 09:33 +0000
            Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-12 12:57 +0200
              Re: Not closing a SQL(ite3) connection Andreas Leitgeb <avl@logic.at> - 2017-08-12 13:27 +0000
                Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-12 16:37 +0200
              Re: Not closing a SQL(ite3) connection Arne Vajhøj <arne@vajhoej.dk> - 2017-08-12 09:55 -0400
                Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-12 16:44 +0200
                  Re: Not closing a SQL(ite3) connection Arne Vajhøj <arne@vajhoej.dk> - 2017-08-12 21:45 -0400
              Re: Not closing a SQL(ite3) connection Martin Gregorie <martin@address-in-sig.invalid> - 2017-08-12 21:04 +0000
                Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-13 00:30 +0200
                  Re: Not closing a SQL(ite3) connection Martin Gregorie <martin@address-in-sig.invalid> - 2017-08-13 11:34 +0000
                    Re: Not closing a SQL(ite3) connection Cecil Westerhof <Cecil@decebal.nl> - 2017-08-13 15:27 +0200
            Re: Not closing a SQL(ite3) connection Andreas Leitgeb <avl@logic.at> - 2017-08-12 13:06 +0000

Page 1 of 2  [1] 2  Next page →

#37106 — Not closing a SQL(ite3) connection

Normally I always close a connection when finished. But I now wrote my
own class SQLite3 to handle things in this class. It has its own
connection. Can there be a problem when a connection is not closed, or
is that not something to worry myself about?

Would it be a good idea to write a function to close the database
connection?

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [next] | [standalone]

#37118

On 8/9/2017 5:28 AM, Cecil Westerhof wrote:
> Normally I always close a connection when finished. But I now wrote my
> own class SQLite3 to handle things in this class. It has its own
> connection. Can there be a problem when a connection is not closed, or
> is that not something to worry myself about?
> 
> Would it be a good idea to write a function to close the database
> connection?

If the class implements the JDBC standard interfaces it will need
a close method to compile.

But even if it does not then I will recommend adding a close
method *and* implement AutoCloseable.

It is the right way to handle database connections.

In some contexts like SE app with embedded DB it is not likely
to make a difference, but it is still a good to do things the right
way. To maintain good habits. And to prevent disasters if the
context suddenly change (that sometimes happens).

Arne

[toc] | [prev] | [next] | [standalone]

#37131

Arne Vajhøj <arne@vajhoej.dk> writes:

> On 8/9/2017 5:28 AM, Cecil Westerhof wrote:
>> Normally I always close a connection when finished. But I now wrote my
>> own class SQLite3 to handle things in this class. It has its own
>> connection. Can there be a problem when a connection is not closed, or
>> is that not something to worry myself about?
>>
>> Would it be a good idea to write a function to close the database
>> connection?
>
> If the class implements the JDBC standard interfaces it will need
> a close method to compile.

That is a bit to much for me. ;-) It is just a wrapper around the
SQLite JDBC class.


> But even if it does not then I will recommend adding a close
> method *and* implement AutoCloseable.

I made the class AutoClosable. I use it during the whole run, but
maybe someone else will use it in a try with resources.


> It is the right way to handle database connections.
>
> In some contexts like SE app with embedded DB it is not likely
> to make a difference, but it is still a good to do things the right
> way. To maintain good habits. And to prevent disasters if the
> context suddenly change (that sometimes happens).

Completely my idea. I am often told: “That is not necessary.”, but in
my experience there is much more harm done by ‘not doing what is not
necessary’ as by ‘doing more as necessary’.


I have a strange problem. I made a class SQLite3ResultSet. There is a
lot of stuff to be removed, but that is just to find out if it is
doing what I think it is doing.

In this class I have:
    private ResultSet resultSet = null;
    private Statement stmt      = null;

The constructor is:
    public SQLite3ResultSet(Connection conn, String query) throws SQLite3Exception {
        System.out.println("SQLite3ResultSet: constructor.");
        try {
            stmt      = conn.createStatement();
            resultSet = stmt.executeQuery(query);
        } catch (SQLException e1) {
            String errorMsg = "";

            if (stmt != null) {
                try {
                    stmt.close();
                    stmt = null;
                } catch (SQLException e2) {
                    errorMsg += "Could not close Statement.";
                }
            }
            throw new SQLite3Exception(errorMsg +
                                       "Could not create a resultSet.");
        }
    }

And the close method is (for now, because I want to test it):
    @Override
    public void close() throws SQLite3Exception {
        System.out.println("SQLite3ResultSet: closing open resources.");
        String    errorMsg  = "";

        try {
            stmt.close();
            resultSet.next();
            resultSet.close();
        } catch (SQLException e) {
            errorMsg += "Could not close ResultSet.\n";
            System.out.println(errorMsg);
        }
        System.out.println("Closed resultSet.");
        try {
            stmt.close();
        } catch (SQLException e) {
            errorMsg += "Could not close Statement belonging to ResultSet.\n";
            System.out.println(errorMsg);
        }
        System.out.println("closed stmt");
        System.out.println("SQLite3ResultSet: closed open resources.");
        if (!errorMsg.isEmpty()) {
            throw new SQLite3Exception(errorMsg);
        }
    }

In the first try I close the stmt and then I do a resultSet.next(). I
would expect an exception, because I closed the stmt, but I do not get
an exception. Why not?

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37136

On 8/11/2017 4:26 AM, Cecil Westerhof wrote:
> I have a strange problem. I made a class SQLite3ResultSet. There is a
> lot of stuff to be removed, but that is just to find out if it is
> doing what I think it is doing.

> And the close method is (for now, because I want to test it):
>      @Override
>      public void close() throws SQLite3Exception {
>          System.out.println("SQLite3ResultSet: closing open resources.");
>          String    errorMsg  = "";
> 
>          try {
>              stmt.close();
>              resultSet.next();
>              resultSet.close();

resultSet.close();
stmt.close();

was rather obvious.

:-)

>          } catch (SQLException e) {
>              errorMsg += "Could not close ResultSet.\n";
>              System.out.println(errorMsg);
>          }
>          System.out.println("Closed resultSet.");
>          try {
>              stmt.close();
>          } catch (SQLException e) {
>              errorMsg += "Could not close Statement belonging to ResultSet.\n";
>              System.out.println(errorMsg);
>          }
>          System.out.println("closed stmt");
>          System.out.println("SQLite3ResultSet: closed open resources.");
>          if (!errorMsg.isEmpty()) {
>              throw new SQLite3Exception(errorMsg);
>          }
>      }
> 
> In the first try I close the stmt and then I do a resultSet.next(). I
> would expect an exception, because I closed the stmt, but I do not get
> an exception. Why not?

There is nothing in documentation for next() requiring an exception to
be thrown (ResultSet is not closed and it may not get a database error -
maybe it does not even talk to the database).

Weird code can give weird outcome, because the author of the
code may never have considered the case.

Arne

[toc] | [prev] | [next] | [standalone]

#37138

Arne Vajhøj <arne@vajhoej.dk> writes:

> On 8/11/2017 4:26 AM, Cecil Westerhof wrote:
>> I have a strange problem. I made a class SQLite3ResultSet. There is a
>> lot of stuff to be removed, but that is just to find out if it is
>> doing what I think it is doing.

You did not read this part?


>> And the close method is (for now, because I want to test it):

And also not this?


>>      @Override
>>      public void close() throws SQLite3Exception {
>>          System.out.println("SQLite3ResultSet: closing open resources.");
>>          String    errorMsg  = "";
>>
>>          try {
>>              stmt.close();
>>              resultSet.next();
>>              resultSet.close();
>
> resultSet.close();
> stmt.close();
>
> was rather obvious.

If you read my message a bit better, you would have seen that it is
not real code, but that I do things that are wrong to verify that the
good code does what I think it does.


>>          } catch (SQLException e) {
>>              errorMsg += "Could not close ResultSet.\n";
>>              System.out.println(errorMsg);
>>          }
>>          System.out.println("Closed resultSet.");
>>          try {
>>              stmt.close();
>>          } catch (SQLException e) {
>>              errorMsg += "Could not close Statement belonging to ResultSet.\n";
>>              System.out.println(errorMsg);
>>          }
>>          System.out.println("closed stmt");
>>          System.out.println("SQLite3ResultSet: closed open resources.");
>>          if (!errorMsg.isEmpty()) {
>>              throw new SQLite3Exception(errorMsg);
>>          }
>>      }
>>
>> In the first try I close the stmt and then I do a resultSet.next(). I
>> would expect an exception, because I closed the stmt, but I do not get
>> an exception. Why not?
>
> There is nothing in documentation for next() requiring an exception to
> be thrown (ResultSet is not closed and it may not get a database error -
> maybe it does not even talk to the database).

That was the problem. When using:
    resultSet.getInt("Count");
instead of:
    resultSet.next();

I get the error I expected.


Now I can remove the stmt field out of my class.

With this I verified that I did not get a Statement leak:
    @Override
    public void close() throws SQLite3Exception {
        System.out.println("SQLite3ResultSet: closing open resources.");
        try {
            Statement stmt = resultSet.getStatement();

            stmt.close();
            System.out.println("Closed stmt.");
            resultSet.getInt("Count");
            System.out.println("Did getInt");
            resultSet.close();
        } catch (SQLException e) {
            throw new SQLite3Exception("Could not close ResultSet.");
        }
        System.out.println("Closed resultSet.");
    }

I do get ‘Closed stmt, but I do not get ‘Did getInt’. So the closing
of stmt works.

The real method is now:
    @Override
    public void close() throws SQLite3Exception {
        boolean closedResultSet = false;

        try {
            Statement stmt = resultSet.getStatement();

            resultSet.close();
            stmt.close();
        } catch (SQLException e) {
            if (closedResultSet) {
                throw new SQLite3Exception("Could not close Statement after closing ResultSet.");
            }
            throw new SQLite3Exception("Could not close ResultSet.");
        }
        System.out.println("Closed resultSet.");
    }

The reason that I wrote this code was that I do not want to have to
put a Statement and a ResultSet into my try with resources.

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37139

Cecil Westerhof <Cecil@decebal.nl> wrote:
> You did not read this part?
> And also not this?

The net is a busy place.  Sometimes, people here do a simple
"optimization" based on the likelihood of helping divided by
effort:

-) reading a longish description (prose) is "much effort".
-) spotting some fishy code constellation is little effort.

In many cases, the fishy code constellation is already the
problem, so effort is saved by not reading the prose.

In those rare cases where the prose happens to be essential
and an answer thus ends up not helpful, it is expected that
the OP will say so and also shorten the prose - afterall
it's him who wants more helpful replies.

PS: Didn't read this prose? Doesn't matter. It was still less 
effort to write, than to wrap my head around the motivation
for each single method. ;-)

[toc] | [prev] | [next] | [standalone]

#37140

Andreas Leitgeb <avl@logic.at> writes:

> Cecil Westerhof <Cecil@decebal.nl> wrote:
>> You did not read this part?
>> And also not this?
>
> The net is a busy place.  Sometimes, people here do a simple
> "optimization" based on the likelihood of helping divided by
> effort:
>
> -) reading a longish description (prose) is "much effort".
> -) spotting some fishy code constellation is little effort.
>
> In many cases, the fishy code constellation is already the
> problem, so effort is saved by not reading the prose.
>
> In those rare cases where the prose happens to be essential
> and an answer thus ends up not helpful, it is expected that
> the OP will say so and also shorten the prose - afterall
> it's him who wants more helpful replies.
>
> PS: Didn't read this prose? Doesn't matter. It was still less 
> effort to write, than to wrap my head around the motivation
> for each single method. ;-)

I try to keep that in mind next time.

By the way: you did put me in the right direction. (I should have said
that also.)

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37145

On 8/12/2017 4:00 AM, Cecil Westerhof wrote:
> Arne Vajhøj <arne@vajhoej.dk> writes:
>> On 8/11/2017 4:26 AM, Cecil Westerhof wrote:
>>> I have a strange problem. I made a class SQLite3ResultSet. There is a
>>> lot of stuff to be removed, but that is just to find out if it is
>>> doing what I think it is doing.
> 
> You did not read this part?

I did.

>>> And the close method is (for now, because I want to test it):
> 
> And also not this?

I did.

>>>       @Override
>>>               stmt.close();
>>>               resultSet.next();
>>>               resultSet.close();
>>
>> resultSet.close();
>> stmt.close();
>>
>> was rather obvious.
> 
> If you read my message a bit better, you would have seen that it is
> not real code, but that I do things that are wrong to verify that the
> good code does what I think it does.

I got that this was test code.

But it was not obvious to me exactly what was the test and what
was context.

>>> In the first try I close the stmt and then I do a resultSet.next(). I
>>> would expect an exception, because I closed the stmt, but I do not get
>>> an exception. Why not?
>>
>> There is nothing in documentation for next() requiring an exception to
>> be thrown (ResultSet is not closed and it may not get a database error -
>> maybe it does not even talk to the database).
> 
> That was the problem. When using:
>      resultSet.getInt("Count");
> instead of:
>      resultSet.next();
> 
> I get the error I expected.

Calling a getXxxx before next should give an error even if
statement was not closed.

> Now I can remove the stmt field out of my class.
> 
> With this I verified that I did not get a Statement leak:
>      @Override
>      public void close() throws SQLite3Exception {
>          System.out.println("SQLite3ResultSet: closing open resources.");
>          try {
>              Statement stmt = resultSet.getStatement();
> 
>              stmt.close();
>              System.out.println("Closed stmt.");
>              resultSet.getInt("Count");
>              System.out.println("Did getInt");
>              resultSet.close();
>          } catch (SQLException e) {
>              throw new SQLite3Exception("Could not close ResultSet.");
>          }
>          System.out.println("Closed resultSet.");
>      }
> 
> I do get ‘Closed stmt, but I do not get ‘Did getInt’. So the closing
> of stmt works.

Unusual way to test that. But yes.

> The real method is now:
>      @Override
>      public void close() throws SQLite3Exception {
>          boolean closedResultSet = false;
> 
>          try {
>              Statement stmt = resultSet.getStatement();
> 
>              resultSet.close();

There mys be missing an assignment here.

>              stmt.close();
>          } catch (SQLException e) {
>              if (closedResultSet) {
>                  throw new SQLite3Exception("Could not close Statement after closing ResultSet.");
>              }
>              throw new SQLite3Exception("Could not close ResultSet.");

I don't like this conditional here.

Either separate catch or just let the SQLException propagate information
about what the problem was.

>          }
>          System.out.println("Closed resultSet.");
>      }
> 
> The reason that I wrote this code was that I do not want to have to
> put a Statement and a ResultSet into my try with resources.

I think it is common to use one try for Statement and one
for ResultSet.

Arne

[toc] | [prev] | [next] | [standalone]

#37147

Arne Vajhøj <arne@vajhoej.dk> writes:

> I got that this was test code.
>
> But it was not obvious to me exactly what was the test and what
> was context.

OK, next time I should be more clear,


>> That was the problem. When using:
>>      resultSet.getInt("Count");
>> instead of:
>>      resultSet.next();
>>
>> I get the error I expected.
>
> Calling a getXxxx before next should give an error even if
> statement was not closed.

That is what you would expect, but with SQLite it is not necessary.
But it would better to do it anyway. Even if it is throwaway code.


>> The real method is now:
>>      @Override
>>      public void close() throws SQLite3Exception {
>>          boolean closedResultSet = false;
>>
>>          try {
>>              Statement stmt = resultSet.getStatement();
>>
>>              resultSet.close();
>
> There mys be missing an assignment here.

The resultSet is assigned in the constructor of the class.


>
>>              stmt.close();
>>          } catch (SQLException e) {
>>              if (closedResultSet) {
>>                  throw new SQLite3Exception("Could not close Statement after closing ResultSet.");
>>              }
>>              throw new SQLite3Exception("Could not close ResultSet.");
>
> I don't like this conditional here.
>
> Either separate catch or just let the SQLException propagate information
> about what the problem was.

I want it to be clear when the closing of the resultSet went OK, but
the closing of stmt not. I was thinking about doing it in two tries,
but did not do it. And I should do three tries, because the fetching
of stmt can also go wrong. So now I have:
    @Override
    public void close() throws SQLException {
        Statement stmt;

        try {
            stmt = resultSet.getStatement();
        } catch (SQLException e) {
            SQLException extException = new SQLException("Could not get Statement from ResultSet.");

            extException.addSuppressed(e);
            e = extException;
            throw e;
        }
        resultSet.close();
        try {
            stmt.close();
        } catch (SQLException e) {
            SQLException extException = new SQLException("Could not close Statement after closing ResultSet.");

            extException.addSuppressed(e);
            e = extException;
            throw e;
        }
    }


>> The reason that I wrote this code was that I do not want to have to
>> put a Statement and a ResultSet into my try with resources.
>
> I think it is common to use one try for Statement and one
> for ResultSet.

I know, but I do not like that. And it also does not hurt to just
write something for the fun of it. :-D

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37157

On 8/12/2017 10:26 AM, Cecil Westerhof wrote:
> Arne Vajhøj <arne@vajhoej.dk> writes:
>>>           } catch (SQLException e) {
>>>               if (closedResultSet) {
>>>                   throw new SQLite3Exception("Could not close Statement after closing ResultSet.");
>>>               }
>>>               throw new SQLite3Exception("Could not close ResultSet.");
>>
>> I don't like this conditional here.
>>
>> Either separate catch or just let the SQLException propagate information
>> about what the problem was.
> 
> I want it to be clear when the closing of the resultSet went OK, but
> the closing of stmt not. I was thinking about doing it in two tries,
> but did not do it. And I should do three tries, because the fetching
> of stmt can also go wrong. So now I have:
>      @Override
>      public void close() throws SQLException {
>          Statement stmt;
> 
>          try {
>              stmt = resultSet.getStatement();
>          } catch (SQLException e) {
>              SQLException extException = new SQLException("Could not get Statement from ResultSet.");
> 
>              extException.addSuppressed(e);
>              e = extException;
>              throw e;
>          }
>          resultSet.close();
>          try {
>              stmt.close();
>          } catch (SQLException e) {
>              SQLException extException = new SQLException("Could not close Statement after closing ResultSet.");
> 
>              extException.addSuppressed(e);
>              e = extException;
>              throw e;
>          }
>      }

I would somewhat expect that the cause of the problem be available in
the Exception itself, so one could avoid all this. But if not then ...

Arne

[toc] | [prev] | [next] | [standalone]

#37230

Arne Vajhøj <arne@vajhoej.dk> writes:

>> I want it to be clear when the closing of the resultSet went OK, but
>> the closing of stmt not. I was thinking about doing it in two tries,
>> but did not do it. And I should do three tries, because the fetching
>> of stmt can also go wrong. So now I have:
>>      @Override
>>      public void close() throws SQLException {
>>          Statement stmt;
>>
>>          try {
>>              stmt = resultSet.getStatement();
>>          } catch (SQLException e) {
>>              SQLException extException = new SQLException("Could not get Statement from ResultSet.");
>>
>>              extException.addSuppressed(e);
>>              e = extException;
>>              throw e;
>>          }
>>          resultSet.close();
>>          try {
>>              stmt.close();
>>          } catch (SQLException e) {
>>              SQLException extException = new SQLException("Could not close Statement after closing ResultSet.");
>>
>>              extException.addSuppressed(e);
>>              e = extException;
>>              throw e;
>>          }
>>      }
>
> I would somewhat expect that the cause of the problem be available in
> the Exception itself, so one could avoid all this. But if not then
> ...

You are right, I rewrote it:
    @Override
    public void close() throws SQLException {
        Statement stmt;

        if (isClosed) {
            throw new SQLException("SQLResultSet is already closed.");
        }
        stmt     = resultSet.getStatement();
        resultSet.close();
        stmt.close();
        isClosed = true;
    }

Much cleaner code.

I check for it being closed, because I think it is an error when you
try to close more as once.

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37150

> If you read my message a bit better, you would have seen ...

Supercilious much?

[toc] | [prev] | [next] | [standalone]

#37141

>>          try {
>>              stmt.close();
>>              resultSet.next();
>>              resultSet.close();
>>          } catch (SQLException e) {
>>              errorMsg += "Could not close ResultSet.\n";
>>              System.out.println(errorMsg);
>>          }

Also, it might be better to actually add the exception information
rather than a hardcoded string to errorMsg. Unless you really ran it 
in the debugger and examined e, you wouldn't know, what really 
threw the exception.

[toc] | [prev] | [next] | [standalone]

#37142

Andreas Leitgeb <avl@logic.at> writes:

>>>          try {
>>>              stmt.close();
>>>              resultSet.next();
>>>              resultSet.close();
>>>          } catch (SQLException e) {
>>>              errorMsg += "Could not close ResultSet.\n";
>>>              System.out.println(errorMsg);
>>>          }
>
> Also, it might be better to actually add the exception information
> rather than a hardcoded string to errorMsg. Unless you really ran it 
> in the debugger and examined e, you wouldn't know, what really 
> threw the exception.

This was only for testing purposes. My real code has become:
    @Override
    public void close() throws SQLException {
        boolean closedResultSet = false;

        try {
            Statement stmt = resultSet.getStatement();

            resultSet.close();
            closedResultSet = true;
            stmt.close();
        } catch (SQLException e) {
            if (closedResultSet) {
                SQLException extException = new SQLException("Could not close Statement after closing ResultSet.");

                extException.addSuppressed(e);
                e = extException;
            }
            throw e;
        }
    }

The String will be done with a variable of-course. Beside that: is the
code OK, or can it be improved?

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37144

Cecil Westerhof <Cecil@decebal.nl> wrote:
> Andreas Leitgeb <avl@logic.at> writes:
>>>>          try {
>>>>              stmt.close();
>>>>              resultSet.next();
>>>>              resultSet.close();
>>>>          } catch (SQLException e) {
>>>>              errorMsg += "Could not close ResultSet.\n";
>>>>              System.out.println(errorMsg);
>>>>          }
> This was only for testing purposes.

I was aware of that. My point was, that the way your test code
looked like, it was likely to mislead you to wrong conclusions,
about what really caused the exception.

See also my other f-up, which I started editing, then forgot, and
some time later got back to and sent off, before seeing this post.

> My real code has become:
>         try {
>             Statement stmt = resultSet.getStatement();
>             resultSet.close();
>             closedResultSet = true;
>             stmt.close();

That's a meaning of "real code", that I'd consider rather odd ;-)
It's probably more like: the new "real" test code ...

I'm not too sure though, about the value of this test.

If the resultSet is already closed, then retrieving the Statement 
from it might already fail (and the result might make you think that
closing the resultset threw, when it might just as well have been
already the .getStatement(); - both before setting the boolean.
Maybe my other followup might still be in principle relevant.

If instead the resultSet was still open at time of calling your code,
then there shouldn't really be any exception thrown at all.

[toc] | [prev] | [next] | [standalone]

#37148

Andreas Leitgeb <avl@logic.at> writes:

>> My real code has become:
>>         try {
>>             Statement stmt = resultSet.getStatement();
>>             resultSet.close();
>>             closedResultSet = true;
>>             stmt.close();
>
> That's a meaning of "real code", that I'd consider rather odd ;-)
> It's probably more like: the new "real" test code ...

Nope, I think this is real code. But if there is an error, I do not
mind to be told that.

I have now a class SQLResultSet. I now only have to get an instance of
this class instead of an instance of Statement and ResultSet. When the
instance of SQLResultSet is closed, its ResultSet needs to be closed
and its implicit Statement. Otherwise there is a leak.


> If the resultSet is already closed, then retrieving the Statement 
> from it might already fail (and the result might make you think that
> closing the resultset threw, when it might just as well have been
> already the .getStatement(); - both before setting the boolean.
> Maybe my other followup might still be in principle relevant.

That is a good point: what to do if close is called more as once. You
keep me busy. ;-)

I already have a different throw for when the retrieving of Statement
goes wrong. See my other reply.


> If instead the resultSet was still open at time of calling your code,
> then there shouldn't really be any exception thrown at all.

It should not, but when making a library you have to cover all your
bases.


I plan in making it more general also. I initially made it for
SQLite3, but it will not hurt to let it work with ‘all’ databases.

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37146

On 8/12/2017 6:57 AM, Cecil Westerhof wrote:
> This was only for testing purposes. My real code has become:
>      @Override
>      public void close() throws SQLException {
>          boolean closedResultSet = false;
> 
>          try {
>              Statement stmt = resultSet.getStatement();
> 
>              resultSet.close();
>              closedResultSet = true;
>              stmt.close();
>          } catch (SQLException e) {
>              if (closedResultSet) {
>                  SQLException extException = new SQLException("Could not close Statement after closing ResultSet.");
> 
>                  extException.addSuppressed(e);
>                  e = extException;
>              }
>              throw e;
>          }
>      }
> 
> The String will be done with a variable of-course. Beside that: is the
> code OK, or can it be improved?

I see little value in this over the much more simple:

      @Override
      public void close() throws SQLException {
          Statement stmt = resultSet.getStatement();
          resultSet.close();
          stmt.close();
      }

:-)

The original SQLException should contain sufficient information. I
don't see "Could not close Statement after closing ResultSet." adding
value.

Arne

[toc] | [prev] | [next] | [standalone]

#37149

Arne Vajhøj <arne@vajhoej.dk> writes:

> On 8/12/2017 6:57 AM, Cecil Westerhof wrote:
>> This was only for testing purposes. My real code has become:
>>      @Override
>>      public void close() throws SQLException {
>>          boolean closedResultSet = false;
>>
>>          try {
>>              Statement stmt = resultSet.getStatement();
>>
>>              resultSet.close();
>>              closedResultSet = true;
>>              stmt.close();
>>          } catch (SQLException e) {
>>              if (closedResultSet) {
>>                  SQLException extException = new SQLException("Could not close Statement after closing ResultSet.");
>>
>>                  extException.addSuppressed(e);
>>                  e = extException;
>>              }
>>              throw e;
>>          }
>>      }
>>
>> The String will be done with a variable of-course. Beside that: is the
>> code OK, or can it be improved?
>
> I see little value in this over the much more simple:
>
>      @Override
>      public void close() throws SQLException {
>          Statement stmt = resultSet.getStatement();
>          resultSet.close();
>          stmt.close();
>      }
>
> :-)
>
> The original SQLException should contain sufficient information. I
> don't see "Could not close Statement after closing ResultSet." adding
> value.

Maybe I am gold-plating. I want to make sure that it is clear that it
is my function and not a user function where it goes wrong, but
of-course:
    nl.decebal.sql.SQLResultSet.close
makes that clear.

I probably should remove it.

-- 
Cecil Westerhof
Senior Software Engineer
LinkedIn: http://www.linkedin.com/in/cecilwesterhof

[toc] | [prev] | [next] | [standalone]

#37158

On 8/12/2017 10:44 AM, Cecil Westerhof wrote:
> Arne Vajhøj <arne@vajhoej.dk> writes:
>> I see little value in this over the much more simple:
>>
>>       @Override
>>       public void close() throws SQLException {
>>           Statement stmt = resultSet.getStatement();
>>           resultSet.close();
>>           stmt.close();
>>       }
>>
>> :-)
>>
>> The original SQLException should contain sufficient information. I
>> don't see "Could not close Statement after closing ResultSet." adding
>> value.
> 
> Maybe I am gold-plating. I want to make sure that it is clear that it
> is my function and not a user function where it goes wrong, but
> of-course:
>      nl.decebal.sql.SQLResultSet.close
> makes that clear.
> 
> I probably should remove it.

Maybe try some error situations and see how good an
exception text you get with the simple code (note that
you may need to use both stack trace and nested exceptions
to get all available info).

Arne

[toc] | [prev] | [next] | [standalone]

#37151

On Sat, 12 Aug 2017 12:57:40 +0200, Cecil Westerhof wrote:

> The String will be done with a variable of-course. Beside that: is the
> code OK, or can it be improved?
>
The main one is that with most JDBC drivers, the SQLException that 
try...catch intercepts is merely the first SQLEXception in a chain of 
SQLExceptions. You miss a lot of information about what went wrong by not 
repeatedly calling the current SQLException's getNextException() method 
until all members of the chain have been retrieved and their contents 
displayed. Alternatively, call SQLException.iterator and use that to 
iterate through the SQLException chain.
  
If the SQL query was at all complex then the first exception in the chain 
may not be particularly helpful because what caused the query to fail is 
described further down the chain.
 

-- 
martin@   | Martin Gregorie
gregorie. | Essex, UK
org       |

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | comp.lang.java.programmer

csiph-web