Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Message-ID: <7138241.1153142116234.JavaMail.jira@brutus>
Date: Mon, 17 Jul 2006 06:15:16 -0700 (PDT)
From: "Knut Anders Hatlen (JIRA)" <derby-dev@db.apache.org>
To: derby-dev@db.apache.org
Subject: [jira] Commented: (DERBY-1417) Add new, lengthless overloads to the
 streaming api
In-Reply-To: <5577812.1150385909863.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

    [ 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