Derby
  1. Derby
  2. 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.

    Details

    • Urgency:
      Low
    • Issue & fix info:
      Newcomer

      Description

      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();

      1. d129-1a.diff
        3.02 MB
        Knut Anders Hatlen
      2. d129-1b.diff
        3.02 MB
        Knut Anders Hatlen
      3. delta-1a-1b.diff
        5 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Myrna van Lunteren added a comment -

          bulk change to close all issues resolved but not closed and not changed since June 1, 2014.

          Show
          Myrna van Lunteren added a comment - bulk change to close all issues resolved but not closed and not changed since June 1, 2014.
          Hide
          Knut Anders Hatlen added a comment -

          The release note for DERBY-5749 covered this issue too.

          Show
          Knut Anders Hatlen added a comment - The release note for DERBY-5749 covered this issue too.
          Hide
          Kathey Marsden added a comment -

          It seems like this issue would not be appropriate for backporting because of the behavior change. Reopening to add label. I wonder should there be a release note?

          Show
          Kathey Marsden added a comment - It seems like this issue would not be appropriate for backporting because of the behavior change. Reopening to add label. I wonder should there be a release note?
          Hide
          Knut Anders Hatlen added a comment -

          Thanks again, Dag. Committed revision 1341046. I'll update the DERBY-5749 release note with information about the warning.

          Show
          Knut Anders Hatlen added a comment - Thanks again, Dag. Committed revision 1341046. I'll update the DERBY-5749 release note with information about the warning.
          Hide
          Dag H. Wanvik added a comment -

          +1 to merging release notes with DERBY-5749

          Show
          Dag H. Wanvik added a comment - +1 to merging release notes with DERBY-5749
          Hide
          Dag H. Wanvik added a comment -

          Thanks for the new patch, Knut. The changes look good! +1 Also, +1 to fixing getSqlCode in a separate JIRA.

          Show
          Dag H. Wanvik added a comment - Thanks for the new patch, Knut. The changes look good! +1 Also, +1 to fixing getSqlCode in a separate JIRA.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a new patch, d129-1b.diff, that addresses Dag's review comments. I've also updloaded a patch delta-1a-1b.diff, which only shows the changes between 1a and 1b.

          The regression tests passed, except two failures in UpdateStatisticsTest, which I think are unrelated to the changes in the patch. Will log a separate bug for those failures.

          Show
          Knut Anders Hatlen added a comment - Attaching a new patch, d129-1b.diff, that addresses Dag's review comments. I've also updloaded a patch delta-1a-1b.diff, which only shows the changes between 1a and 1b. The regression tests passed, except two failures in UpdateStatisticsTest, which I think are unrelated to the changes in the patch. Will log a separate bug for those failures.
          Hide
          Knut Anders Hatlen added a comment -

          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.

          Show
          Knut Anders Hatlen added a comment - 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.
          Hide
          Dag H. Wanvik added a comment -

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

          Some small nits:

          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.

          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.

          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.

          • 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?

          • 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 SQLEWarning"?

          Show
          Dag H. Wanvik added a comment - Hi Knut, thanks for this cleanup! I hope this can go in along with DERBY-5749 into 10.9 (both concern truncation diagnostics). Some small nits: 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. 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. 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. 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? 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 SQLEWarning"?
          Hide
          Knut Anders Hatlen added a comment -

          One other thing that's probably worth mentioning, is that the
          truncation warning doesn't always end up on the same row as the one
          where the truncation happened. This seems to be the case when there's
          been a join or the result has been sorted.

          For example:

          ij> create table t1 (x varchar(5));
          0 rows inserted/updated/deleted
          ij> insert into t1 values 'hello', 'hi', 'hey', 'yo';
          4 rows inserted/updated/deleted
          ij> select cast(x as char(3)) from t1 order by x desc;
          1


          yo
          WARNING 01004: Data truncation
          hi
          hey
          hel

          4 rows selected

          Here, the warning comes with the first row, whereas the truncated
          value is in the last row.

          This appears to be an existing problem with warnings in Derby, also
          seen with other types of warnings. For example, with 10.8.2.2, I see
          this in a query that generates a warning of a different kind:

          ij> create table t2(x int, y int);
          0 rows inserted/updated/deleted
          ij> insert into t2 values (1,1),(2,2),(3,null);
          3 rows inserted/updated/deleted
          ij> select x, avg from t2 group by x order by x;
          X |2
          -----------------------
          1 |1
          WARNING 01003: Null values were eliminated from the argument of a column function.
          2 |2
          3 |NULL

          3 rows selected

          Here, too, the warning comes with the first row, whereas the row
          containing the NULL value that it warns about, shows up in the last
          row. So the problem isn't introduced by the patch.

          Show
          Knut Anders Hatlen added a comment - One other thing that's probably worth mentioning, is that the truncation warning doesn't always end up on the same row as the one where the truncation happened. This seems to be the case when there's been a join or the result has been sorted. For example: ij> create table t1 (x varchar(5)); 0 rows inserted/updated/deleted ij> insert into t1 values 'hello', 'hi', 'hey', 'yo'; 4 rows inserted/updated/deleted ij> select cast(x as char(3)) from t1 order by x desc; 1 yo WARNING 01004: Data truncation hi hey hel 4 rows selected Here, the warning comes with the first row, whereas the truncated value is in the last row. This appears to be an existing problem with warnings in Derby, also seen with other types of warnings. For example, with 10.8.2.2, I see this in a query that generates a warning of a different kind: ij> create table t2(x int, y int); 0 rows inserted/updated/deleted ij> insert into t2 values (1,1),(2,2),(3,null); 3 rows inserted/updated/deleted ij> select x, avg from t2 group by x order by x; X |2 ----------------------- 1 |1 WARNING 01003: Null values were eliminated from the argument of a column function. 2 |2 3 |NULL 3 rows selected Here, too, the warning comes with the first row, whereas the row containing the NULL value that it warns about, shows up in the last row. So the problem isn't introduced by the patch.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching patch d129-1a.diff, which is based on warn.diff attached to
          DERBY-5537 with additional code to make the network server propagate
          the warning to the client, and changes to make the tests pass.

          Detailed description of the changes:

          • iapi/types/SQLBinary.java
          • iapi/types/SQLChar.java

          Added code to generate the DataTruncation warning.

          One thing to note is that the javadoc for java.sql.DataTruncation says
          the data size and the transfer size are to be specified in number of
          bytes, so SQLChar needed some logic to convert character length to
          byte length. I refactored the writeUTF() method in that class in order
          to avoid duplication of the bit-fiddling logic when calculating the
          length.

          Another thing to note is that we don't know the index of the column
          that's being truncated, so the index field of the DataTruncation
          warning is set to -1, as DataTruncation's javadoc says getIndex() may
          return -1 if the column index is unknown.

          • iapi/types/SQLBit.java
          • iapi/types/SQLBlob.java
          • iapi/types/SQLVarbit.java

          Use the shared truncate() method added to SQLBinary to truncate and,
          if required, add a warning.

          iapi/sql/ResultSet.java
          impl/sql/execute/BasicNoPutResultSetImpl.java
          impl/sql/execute/NoRowsResultSetImpl.java
          impl/sql/execute/TemporaryRowHolderResultSet.java

          Changes to allow adding the warning even if we don't end up returning
          a JDBC ResultSet, in which case the warning will be added to the
          executing statement.

          • impl/drda/DRDAConnThread.java

          DataTruncation.getErrorCode() always returns 0 (there's no way to
          change that), but the protocol uses code 0 to indicate that the client
          should discard the warning. The patch makes the server send error code
          10000 (ExceptionSeverity.WARNING_SEVERITY) for DataTruncation, like it
          does for all other warnings.

          • iapi/services/io/CounterOutputStream.java

          Modified to allow it to be used to count number of bytes in a string
          when generating DataTruncation warning in SQLChar. Its javadoc says
          the stream will discard bytes written to it if it's created with the
          zero-arg constructor. However, it does not discard the bytes and fails
          with a NPE when trying to write to a null stream when used this way.
          The patch modifies the write methods to do what the javadoc says they
          should (and removes one of them - write(byte[]), since the default
          method inherited from OutputStream does the right thing, that is, it
          forwards the calls to write(byte[],int,int)).

          • shared/common/reference/SQLState.java

          Renamed constant from LANG_FILE_ERROR to LANG_IO_EXCEPTION since the
          corresponding error message doesn't necessarily have to do with files.
          Since the message wasn't actually in use before the patch, even though
          it existed in the code, no callers had to be changed because of this.
          The patch makes use of the message in SQLChar.getUTF8Length().

          • functionTests/tests/lang/CastingTest.java

          Added a test case that verified that DataTruncation warnings were
          added correctly to ResultSets and Statements. Enabled the test for
          client/server mode as well to verify that the warnings made it over to
          the client.

          Additionally, lots of canons had to be updated because of new
          warnings. Most of these were the expected truncation warnings, but
          there were also some new warnings of the type

          WARNING 01003: Null values were eliminated from the argument of a column function.

          that appeared because of the changes that propagated warnings from
          sub-queries of DML statements up to the executing statement. The new
          warnings looked correct to me.

          The canons were either changed to expect the new warnings, or, in the
          cases where the new warnings added too much noise (in particular the
          old framework tests that include LockTableQuery.subsql), the queries
          were changed so that they would not emit DataTruncation warnings.

          Show
          Knut Anders Hatlen added a comment - Attaching patch d129-1a.diff, which is based on warn.diff attached to DERBY-5537 with additional code to make the network server propagate the warning to the client, and changes to make the tests pass. Detailed description of the changes: iapi/types/SQLBinary.java iapi/types/SQLChar.java Added code to generate the DataTruncation warning. One thing to note is that the javadoc for java.sql.DataTruncation says the data size and the transfer size are to be specified in number of bytes, so SQLChar needed some logic to convert character length to byte length. I refactored the writeUTF() method in that class in order to avoid duplication of the bit-fiddling logic when calculating the length. Another thing to note is that we don't know the index of the column that's being truncated, so the index field of the DataTruncation warning is set to -1, as DataTruncation's javadoc says getIndex() may return -1 if the column index is unknown. iapi/types/SQLBit.java iapi/types/SQLBlob.java iapi/types/SQLVarbit.java Use the shared truncate() method added to SQLBinary to truncate and, if required, add a warning. iapi/sql/ResultSet.java impl/sql/execute/BasicNoPutResultSetImpl.java impl/sql/execute/NoRowsResultSetImpl.java impl/sql/execute/TemporaryRowHolderResultSet.java Changes to allow adding the warning even if we don't end up returning a JDBC ResultSet, in which case the warning will be added to the executing statement. impl/drda/DRDAConnThread.java DataTruncation.getErrorCode() always returns 0 (there's no way to change that), but the protocol uses code 0 to indicate that the client should discard the warning. The patch makes the server send error code 10000 (ExceptionSeverity.WARNING_SEVERITY) for DataTruncation, like it does for all other warnings. iapi/services/io/CounterOutputStream.java Modified to allow it to be used to count number of bytes in a string when generating DataTruncation warning in SQLChar. Its javadoc says the stream will discard bytes written to it if it's created with the zero-arg constructor. However, it does not discard the bytes and fails with a NPE when trying to write to a null stream when used this way. The patch modifies the write methods to do what the javadoc says they should (and removes one of them - write(byte[]), since the default method inherited from OutputStream does the right thing, that is, it forwards the calls to write(byte[],int,int)). shared/common/reference/SQLState.java Renamed constant from LANG_FILE_ERROR to LANG_IO_EXCEPTION since the corresponding error message doesn't necessarily have to do with files. Since the message wasn't actually in use before the patch, even though it existed in the code, no callers had to be changed because of this. The patch makes use of the message in SQLChar.getUTF8Length(). functionTests/tests/lang/CastingTest.java Added a test case that verified that DataTruncation warnings were added correctly to ResultSets and Statements. Enabled the test for client/server mode as well to verify that the warnings made it over to the client. Additionally, lots of canons had to be updated because of new warnings. Most of these were the expected truncation warnings, but there were also some new warnings of the type WARNING 01003: Null values were eliminated from the argument of a column function. that appeared because of the changes that propagated warnings from sub-queries of DML statements up to the executing statement. The new warnings looked correct to me. The canons were either changed to expect the new warnings, or, in the cases where the new warnings added too much noise (in particular the old framework tests that include LockTableQuery.subsql), the queries were changed so that they would not emit DataTruncation warnings.
          Hide
          Knut Anders Hatlen added a comment -

          I don't think Jeremy suggested that this issue was invalid, only that the appropriate response is a warning and not an exception. Derby doesn't currently give a warning.

          Show
          Knut Anders Hatlen added a comment - I don't think Jeremy suggested that this issue was invalid, only that the appropriate response is a warning and not an exception. Derby doesn't currently give a warning.
          Hide
          Myrna van Lunteren added a comment -

          [10.5.2 Triage] Mamta, do you agree with Jeremy's comments? If so, can this oldy be closed as Invalid?

          Show
          Myrna van Lunteren added a comment - [10.5.2 Triage] Mamta, do you agree with Jeremy's comments? If so, can this oldy be closed as Invalid?
          Hide
          Jeremy Boynes added a comment -

          I don't believe Derby should throw an exception under these circumstances.

          According to the SQL spec (SQL-03 4.2.1), "if a retrieval assignment or evaluation of a
          <cast specification> would result in the loss of characters due to truncation, then a warning condition is raised." And according to JDBC (3.0 8.2) "when data truncation occurs on a read from the data source, a SQLWarning is reported." The SQL spec considers this a retrieve/cast operation so instead of throwing an exception we should complete execution of the statement and add a java.sql.DataTruncation warning to the list returned from Statement.getWarnings()

          In contrast, if the truncation occurs during a write operation then we do need to throw an exception; we actually do this, throwing an SQLException with SQLState 22001. However, I believe this is not compliant with JDBC which states that "when data truncation occurs on a write to the data source, a DataTruncation object is thrown." So instead of throwing the SQLException base class we should be throwing a DataTruncation subclass.

          To clarify, the reason this is a read operation even though we are executing an INSERT is that the data is being used the CAST function not directly in the INSERT. So if we have a table with a single VARCHAR(3) column

          INSERT INTO TEST VALUES (CAST('01234' AS VARCHAR(3)))

          should complete with a warning but

          INSERT INTO TEST VALUES ('01234')

          should fail by throwing a DataTruncation exception.

          Show
          Jeremy Boynes added a comment - I don't believe Derby should throw an exception under these circumstances. According to the SQL spec (SQL-03 4.2.1), "if a retrieval assignment or evaluation of a <cast specification> would result in the loss of characters due to truncation, then a warning condition is raised." And according to JDBC (3.0 8.2) "when data truncation occurs on a read from the data source, a SQLWarning is reported." The SQL spec considers this a retrieve/cast operation so instead of throwing an exception we should complete execution of the statement and add a java.sql.DataTruncation warning to the list returned from Statement.getWarnings() In contrast, if the truncation occurs during a write operation then we do need to throw an exception; we actually do this, throwing an SQLException with SQLState 22001. However, I believe this is not compliant with JDBC which states that "when data truncation occurs on a write to the data source, a DataTruncation object is thrown." So instead of throwing the SQLException base class we should be throwing a DataTruncation subclass. To clarify, the reason this is a read operation even though we are executing an INSERT is that the data is being used the CAST function not directly in the INSERT. So if we have a table with a single VARCHAR(3) column INSERT INTO TEST VALUES (CAST('01234' AS VARCHAR(3))) should complete with a warning but INSERT INTO TEST VALUES ('01234') should fail by throwing a DataTruncation exception.

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Mamta A. Satoor
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development