Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Date: Mon, 21 May 2012 11:41:42 +0000 (UTC)
From: "Knut Anders Hatlen (JIRA)" <jira@apache.org>
To: derby-dev@db.apache.org
Message-ID: <1303606423.2946.1337600502184.JavaMail.jiratomcat@issues-vm>
Subject: [jira] [Commented] (DERBY-129) Derby should throw a truncation
 error or warning when CASTing a parameter/constant to char or char for bit
 datatypes and the data is too large for the datatype.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/DERBY-129?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13280095#comment-13280095 ] 

Knut Anders Hatlen commented on DERBY-129:
------------------------------------------

Thanks for reviewing the patch, Dag!

> Hi Knut, thanks for this cleanup! I hope this can go in along with
> DERBY-5749 into 10.9 (both concern truncation diagnostics).

If it's completed in time for 10.9, perhaps the warnings should be
mentioned in the DERBY-5749 release note? Might be easier to explain
the two together in one release note than in two separate notes.

> SQLBinary#truncate
>
> - If value is lob, the call to first getLength, then to getValue, can
>   cause us the risk of havin to read a lob twice. If we call getValue
>   first, we can save the call to getLength, no?
>
>   Or, in all of SQLBit, SQLVarBit and SQLBlob, the actual width is
>   actually available at the call site, so it could be passed in
>   instead of doing another getLength? But maybe the lob is already in
>   memory, so LOB reading cannot take place here.. if so, a comment
>   that the getLength should be cheap would be nice.

I think the getLength() calls would typically be cheap because, as you
observed, it has already been called before truncate(), and length is
in most cases cached. The exception seems to be if the SQLBlob wraps a
java.sql.Blob instance, in which case Blob.length() will be called
both times, and there's no guarantee that the Blob class in question
(which could be a user-defined class) is caching the length.

I'll change the signature of truncate() to take the original length as
a parameter, as you suggested, to avoid the extra getLength() calls.

> SQLChar#getUTF8Length
>
> - I think we could use the less specialized class ObjectOutputStream
>   instead of FormatIdOutputStream here for "cs". The write method
>   isn't overriden by FormatIdOutputStream, though, but it made me wary
>   at first, lest extra bytes were written. writeUTF needs
>   ObjectOutputStream, not FormatIdOutputStream.

That was my first thought, too. However, ObjectOutputStream inserts
some extra bytes at block boundaries (typically, after flush() has
been called, or when some internal block size limit is exceeded),
which made it a bit tricky to count the number of bytes. It might be
possible to compensate for this with some extra logic, but I think
that would just complicate the code for little added benefit.

It's easy to get confused with all the different base-classes and
interfaces with similar names here:

  - writeUTF() takes an ObjectOutput, not an ObjectOutputStream

  - FormatIdOutputStream extends DataOutputStream, not
    ObjectOutputStream

  - DataOutputStream does not implement the ObjectOutput interface,
    and can therefore not be passed to writeUTF()

  - FormatIdOutputStream, which is a specialization of
    DataOutputStream, does implement the ObjectOutput interface

So of the three candidates

  - ObjectOutputStream can be passed to writeUTF(), but gives the
    wrong number of bytes

  - DataOutputStream gives the right number of bytes, but cannot be
    passed to writeUTF()

  - FormatIdOutputStream gives the right number of bytes, and it can
    be passed to writeUTF()

> CastingTest:
>
> - /**
>      * Check the results for the queries in testDataTruncation().
>      */
>     private void checkDataTruncationResult(Statement s, String sql)
>
>   Might be good to document the expected row behavior: 1, 0 and 2
>   warnings for rows 1,2 and three respectively, and 3, 3 and 4 in
>   actual dataSize in the Javadoc.

Will copy that info into the javadoc.

> - checkDataTruncationResult(s,
>   "values (cast('abc' as clob(2)), cast('de' as varchar(2)))," +
>   " (cast('fg' as clob(2)), cast('hi' as varchar(2)))," +
>   " (cast('jkl' as clob(2)), cast('mnop' as varchar(2)))");
>
>   Why not clob(2) in column 2 on these rows? Copy/paste error?

Yes, the intention was to test CLOB(2) in both columns. Will fix.

> - DRDAConnThread
>
>   Is the code path
>
>   private int getSqlCode(int severity)
>   {
>     if (severity == CodePoint.SVRCOD_WARNING) // warning
>        return 100; //CLI likes it
>
>   still relevant now that you test "instanceof SQLWarning"?

I don't think this code is reachable anymore now that we test for
instanceof SQLWarning. SQL code 100 is interpreted as end-of-data by
the client, so I don't think that code is correct in the first place.
(The end-of-data warnings are generated/sent via another code path
from DRDAConnThread.doneData(), and should still end up with SQL code
100 on the client, so I don't expect this change to cause any kind of
problem.)

Although it's now unreachable code, I'd prefer to address that in a
separate issue, as I think the entire getSqlCode() method needs to be
fixed. Currently, it returns SQL code -1 for all exceptions, which
makes getErrorCode() return different values on client and embedded.
Logged DERBY-5773 for this.
                
> Derby should throw a truncation error or warning when CASTing a parameter/constant to char or char for bit datatypes and the data is too large for the datatype.
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: DERBY-129
>                 URL: https://issues.apache.org/jira/browse/DERBY-129
>             Project: Derby
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 10.0.2.1
>            Reporter: Mamta A. Satoor
>            Assignee: Knut Anders Hatlen
>              Labels: derby_triage10_8
>         Attachments: d129-1a.diff
>
>
> Derby doesn't throw a truncation exception/warning when data is too large during casting of constants or parameters to character string or bit string data types. 
> Following is ij example for constants which is too big for the datatype it is getting cast to
> ij> values (cast ('hello' as char(3)));
> 1
> ----
> hel
> 1 row selected
> ij> values (cast (X'0102' as char(1) for bit data));
> 1
> ----
> 01
> 1 row selected
> Following code snippet is when using parameters through a JDBC program
>    s.executeUpdate("create table ct (c CLOB(100K))");
>    //the following Formatters just loads cData with 32700 'c' characters
>    String cData = org.apache.derbyTesting.functionTests.util.Formatters.repeatChar("c",32700);
>    //notice that ? in the preared statement below is bound to length 32672
>    pSt = con.prepareStatement("insert into ct values (cast (? as varchar(32672)))");
>    pSt.setString(1, cData);
>    //Derby doesn't throw an exception at ps.execute time for 32700 characters into 32672 parameter. It silently
>    truncates it to 32672
>    pSt.execute();

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira