[
http://issues.apache.org/jira/browse/DERBY-1417?page=comments#action_12421610 ]
Knut Anders Hatlen commented on DERBY-1417:
-------------------------------------------
Hi Kristian,
I have reviewed your patch. The code changes look good and derbyall
runs cleanly on Sun JVM 1.5.0. I ran the jdbc40 tests, and they
complain because some of the methods are missing from
EmbedCallableStatement40 (they should only throw not supported). Also,
they report that many methods are missing from the Brokered* classes,
but I guess you will add them in a later patch?
When I run 'ant javadoc', I see these warnings:
EmbedResultSet.java:2957: warning - @param argument "parameterIndex" is not a
parameter name.
EmbedResultSet.java:4778: warning - @param argument "columnLabel" is not a
parameter name.
EmbedResultSet.java:4830: warning - @param argument "x" is not a parameter name.
EmbedResultSet.java:4888: warning - @param argument "inputStream" is not a
parameter name.
EmbedResultSet.java:4944: warning - @param argument "inputStream" is not a
parameter name.
EmbedResultSet.java:5004: warning - @param argument "reader" is not a parameter
name.
EmbedResultSet.java:5062: warning - @param argument "reader" is not a parameter
name.
Some more comments/questions:
EmbedResultSet.java:
- javadocs for for updateAsciiStream(), updateBinaryStream(),
updateCharacterStream(), updateBlob() and updateClob() say
"@throws SQLFeatureNotSupportedException if the JDBC driver does
not support this method". Since Derby does support these methods,
that sentence could be removed.
- line exceeding 80 characters in updateCharacterStreamInternal()
- the update* methods start with a switch on colType. Could the
switch be replaced with a call to
DataTypeDescriptor.isJDBCTypeEquivalent() or factored out somehow?
EmbedPreparedStatement.java:
- I feel that the names of the new assert*Conditions methods are a
little confusing. When I read "assert", I first thought they were
used for asserting certain conditions in debug/sane builds. What
about renaming them to check*Conditions?
- javadocs contain "@throws SQLFeatureNotSupportedException" for
methods that are implemented
- line exceeding 80 characters in setBlob(int,Blob)
- assertBlobConditions() and assertClobConditions() have a comment
about DB2 compliance. Since the behaviour it refers to (only allow
setBlob() on BLOB columns and setClob() on CLOB columns) seems to
be exactly as specified by the JDBC spec, I think referring to DB2
confuses more than it clarifies.
ReaderToUTF8Stream.java:
- The constructor can throw an IllegalArgumentException, but it is
not caught anywhere, so it will propagate up to the application as
an IllegalArgumentException, not as an SQLException. Since this
exceptional situation only happens if there is a bug in Derby,
perhaps SanityManager.ASSERT could be used instead?
Tests:
- the new assertEquals() methods could be useful to have in
BaseTestCase
- I think it would be good to test that removal of trailing blanks
in clobs is handled correctly
> Add new, lengthless overloads to the streaming api
> --------------------------------------------------
>
> Key: DERBY-1417
> URL: http://issues.apache.org/jira/browse/DERBY-1417
> Project: Derby
> Issue Type: New Feature
> Components: JDBC
> Affects Versions: 10.2.0.0
> Reporter: Rick Hillegas
> Assigned To: Kristian Waagan
> Fix For: 10.2.0.0
>
> Attachments: derby-1417-01-castsInTests.diff,
> derby-1417-1a-notImplemented.diff, derby-1417-1a-notImplemented.stat,
> derby-1417-2a-rstest-refactor.diff, derby-1417-3a-embimpl-and-tests.diff,
> derby-1417-3a-embimpl-and-tests.stat
>
>
> The JDBC4 Expert Group has approved a new set of overloads for the streaming
> methods. These overloads do not take a length argument. Here are the new
> overloads:
> PreparedStatement.setAsciiStream(int parameterIndex, java.io.InputStream x)
> PreparedStatement.setBinaryStream(int parameterIndex, java.io.InputStream x)
> PreparedStatement.setCharacterStream(int parameterIndex, java.io.Reader
> reader)
> PreparedStatement.setNCharacterStream(int parameterIndex, java.io.Reader
> reader)
> PreparedStatement.setBlob(int parameterIndex, java.io.InputStream inputStream)
> PreparedStatement.setClob(int parameterIndex, java.io.Reader reader)
> PreparedStatement.setNClob(int parameterIndex, java.io.Reader reader)
> CallableStatement.setAsciiStream(java.lang.String parameterName,
> java.io.InputStream x)
> CallableStatement.setBinaryStream(java.lang.String parameterName,
> java.io.InputStream x)
> CallableStatement.setCharacterStream(java.lang.String parameterName,
> java.io.Reader reader)
> CallableStatement.setNCharacterStream(java.lang.String parameterName,
> java.io.Reader reader)
> CallableStatement.setBlob(java.lang.String parameterName, java.io.InputStream
> inputStream)
> CallableStatement.setClob(java.lang.String parameterName, java.io.Reader
> reader)
> CallableStatement.setNClob(java.lang.String parameterName, java.io.Reader
> reader)
> ResultSet.updateAsciiStream(int columnIndex, java.io.InputStream x)
> ResultSet.updateAsciiStream(java.lang.String columnLabel, java.io.InputStream
> x)
> ResultSet.updateBinaryStream(int columnIndex, java.io.InputStream x)
> ResultSet.updateBinaryStream(java.lang.String columnLabel,
> java.io.InputStream x, int length)
> ResultSet.updateCharacterStream(int columnIndex, java.io.Reader x)
> ResultSet.updateCharacterStream(java.lang.String columnLabel, java.io.Reader
> x)
> ResultSet.updateNCharacterStream(int columnIndex, java.io.Reader x)
> ResultSet.updateNCharacterStream(java.lang.String columnLabel, java.io.Reader
> x)
> ResultSet.updateBlob(int columnIndex, java.io.InputStream inputStream)
> ResultSet.updateBlob(java.lang.String columnLabel, java.io.InputStream
> inputStream)
> ResultSet.updateClob(int columnIndex, java.io.Reader reader)
> ResultSet.updateClob(java.lang.String columnLabel, java.io.Reader reader)
> ResultSet.updateNClob(int columnIndex, java.io.Reader reader)
> ResultSet.updateNClob(java.lang.String columnLabel, java.io.Reader reader)
> We should add these new overloads soon so that the build will not break when
> this methods turn up in a published Mustang build.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
