LOBStreamControl shouldn't throw SQLException

• Key: DERBY-3783
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Kristian Waagan
• Reporter: Kristian Waagan
• Component/s: JDBC
• Affects Version/s: 10.4.2.0, 10.5.1.1
• Fix Version/s: 10.4.2.0, 10.5.1.1

Description

LOBStreamControl throws three types of exceptions: IOException, SQLException and StandardException.
All the SQLException are generated/thrown from the code in LOBStreamControl.
At this level of the code, SQLException should not be thrown, as it is more tedious to handle both SQLException and StandardException at higher levels.


I propose to replace SQLException with StandardException in LOBStreamControl. The purpose of this change is consistency, and also results it slightly less code at higher levels.

Comments

[Kristian Waagan]

'derby-3783-1a-dont_throw_sqlexception.diff' makes LOBStreamControl throw StandardException instead of SQLException.
I've tried to change as little as possible, but there was logic in some of the catch blocks that had to be moved.

I've run jdbcapi._Suite without failures, running suites.All and derbyall now.
I plan to commit (or post a new patch) once the tests have completed. Patch ready for review.

[Knut Anders Hatlen]

One small suggestion, while you're at it:

LOBInputStream.java:
+ } else {
+ throw new IOException(se.getMessage());
+ }

To preserve the stack trace: throw (IOException) new IOException(se.getMessage()).initCause(se);

Perhaps we should have a helper method for this, since we seem to need it multiple places in these classes.

[Kristian Waagan]

That's a good idea Knut Anders!
Preserving the stack traces is important.

I forgot to add a comment that I committed patch 'derby-3783-1a-dont_throw_sqlexception.diff' yesterday:
 - trunk : revision 677623
 - 10.4 : revision 677628

I'm leaving the issue open for now.

[Knut Anders Hatlen]

I have created a patch which adds a method newIOException(Throwable cause) to the Util class in impl.jdbc and changes all occurrences of new IOException(e.getMessages()) in that package to use the utility method. All tests in derbyall and suites.All passed with the patch.

[Kristian Waagan]

+1 to commit ' d3783-initCause.diff'.

[Knut Anders Hatlen]

Committed d3783-initCause.diff with revision 678388.

[Kristian Waagan]

Backported 'd3783-initCause.diff' to 10.4 with revision 682803. It went in with the patches for DERBY-3766.