Derby
  1. Derby
  2. DERBY-4965

Boolean to char conversion results in integer

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.7.1.1
    • Fix Version/s: 10.8.1.2
    • Component/s: JDBC
    • Labels:
      None
    • Issue & fix info:
      Release Note Needed, Repro attached
    • Bug behavior facts:
      Deviation from standard

      Description

      Seen when running the Java EE CTS on Derby 10.7.1.1. The following code results in "1" being printed, whereas the expected result is "true":

      PreparedStatement ps = c.prepareStatement("values cast(? as char(10))");
      ps.setObject(1, Boolean.TRUE, Types.CHAR);
      ResultSet rs = ps.executeQuery();
      rs.next();
      System.out.println(rs.getString(1));

      Same seen when using VARCHAR or LONGVARCHAR instead of CHAR, and when using setBoolean() instead of setObject().

      1. derby-4965-1a.diff
        6 kB
        Knut Anders Hatlen
      2. derby-4965-1b.diff
        9 kB
        Knut Anders Hatlen
      3. derby-4965-1c.diff
        12 kB
        Knut Anders Hatlen
      4. update-comments.diff
        2 kB
        Knut Anders Hatlen
      5. releaseNote.html
        5 kB
        Knut Anders Hatlen
      6. releaseNote.html
        5 kB
        Rick Hillegas
      7. releaseNote.html
        5 kB
        Rick Hillegas

        Issue Links

          Activity

          Knut Anders Hatlen created issue -
          Hide
          Knut Anders Hatlen added a comment -

          This happens both with the embedded driver and with the client driver.

          Show
          Knut Anders Hatlen added a comment - This happens both with the embedded driver and with the client driver.
          Knut Anders Hatlen made changes -
          Field Original Value New Value
          Issue & fix info [Repro attached]
          Knut Anders Hatlen made changes -
          Assignee Knut Anders Hatlen [ knutanders ]
          Knut Anders Hatlen made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a patch (1a) that makes ParameterMappingTest expose the bug and changes the code so that the test passes. I'm running the regression tests now.

          The patch makes the following changes:

          1) Tests: Make ParameterMappingTest expect "true" instead of "1" when putting a boolean into a character type column.

          2) Engine: Make SQLChar.setValue(boolean) set "true"/"false" instead of "1"/"0".

          3) Client: Send boolean parameter values as booleans instead of smallints. There was already code to do this in NetStatementRequest, but it didn't seem to be used. I activated this code, but since older servers don't expect parameters to be sent as booleans, I also added a new metadata method to check if this is supported by the server. I need to do some more digging to see if the code was actually used in some situations in 10.7 after all, in which case we need more logic to decide whether or not to use the code.

          4) Server: Let DRDAConnThread know what to do if it receives a parameter value with type equal to DRDAConstants.DRDA_TYPE_NBOOLEAN.

          Show
          Knut Anders Hatlen added a comment - Attaching a patch (1a) that makes ParameterMappingTest expose the bug and changes the code so that the test passes. I'm running the regression tests now. The patch makes the following changes: 1) Tests: Make ParameterMappingTest expect "true" instead of "1" when putting a boolean into a character type column. 2) Engine: Make SQLChar.setValue(boolean) set "true"/"false" instead of "1"/"0". 3) Client: Send boolean parameter values as booleans instead of smallints. There was already code to do this in NetStatementRequest, but it didn't seem to be used. I activated this code, but since older servers don't expect parameters to be sent as booleans, I also added a new metadata method to check if this is supported by the server. I need to do some more digging to see if the code was actually used in some situations in 10.7 after all, in which case we need more logic to decide whether or not to use the code. 4) Server: Let DRDAConnThread know what to do if it receives a parameter value with type equal to DRDAConstants.DRDA_TYPE_NBOOLEAN.
          Knut Anders Hatlen made changes -
          Attachment derby-4965-1a.diff [ 12468009 ]
          Hide
          Knut Anders Hatlen added a comment -

          Some issues with the 1a patch:

          1) Two failures in NullIfTest (expected:<[1]> but was:<[true]>)

          2) trunk client against 10.7 server gives ClassCastException when calling PreparedStatement.setBoolean()

          3) The patch only partially fixes the problem on the client side. It addresses setBoolean(), but not setObject(). It fixes both in the embedded driver

          Show
          Knut Anders Hatlen added a comment - Some issues with the 1a patch: 1) Two failures in NullIfTest (expected:< [1] > but was:< [true] >) 2) trunk client against 10.7 server gives ClassCastException when calling PreparedStatement.setBoolean() 3) The patch only partially fixes the problem on the client side. It addresses setBoolean(), but not setObject(). It fixes both in the embedded driver
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a new patch (1b) that addresses the issues with the 1a patch.

          Description of the changes:

          M java/engine/org/apache/derby/iapi/types/SQLChar.java

          Use true/false when setting the value of an SQLChar from a boolean.

          M java/drda/org/apache/derby/impl/drda/DRDAConnThread.java

          Let the server know what to do if it gets a parameter of type DRDA_TYPE_NBOOLEAN from the client.

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/NullIfTest.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java

          Make the tests expect "true" instead of "1" when setting a character type value from a boolean.

          M java/client/org/apache/derby/client/net/NetStatementRequest.java

          Handle BIT values as BOOLEAN instead of SMALLINT. Send BIT/BOOLEAN as boolean to the server, if it knows how to handle it (above mentioned fix in DRDAConnThread.java is required).

          M java/client/org/apache/derby/client/am/CrossConverters.java

          Add cross conversion rules for conversions from boolean to the various other types (for setObject() with type argument).

          M java/client/org/apache/derby/client/am/DatabaseMetaData.java

          Add new method that tells whether the server understands boolean parameter value (that is, if it has the above mentioned fix in DRDAConnThread.java).

          Derbyall, suites.All and the client/server compatibility tests ran cleanly with the patch.

          Show
          Knut Anders Hatlen added a comment - Attaching a new patch (1b) that addresses the issues with the 1a patch. Description of the changes: M java/engine/org/apache/derby/iapi/types/SQLChar.java Use true/false when setting the value of an SQLChar from a boolean. M java/drda/org/apache/derby/impl/drda/DRDAConnThread.java Let the server know what to do if it gets a parameter of type DRDA_TYPE_NBOOLEAN from the client. M java/testing/org/apache/derbyTesting/functionTests/tests/lang/NullIfTest.java M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java Make the tests expect "true" instead of "1" when setting a character type value from a boolean. M java/client/org/apache/derby/client/net/NetStatementRequest.java Handle BIT values as BOOLEAN instead of SMALLINT. Send BIT/BOOLEAN as boolean to the server, if it knows how to handle it (above mentioned fix in DRDAConnThread.java is required). M java/client/org/apache/derby/client/am/CrossConverters.java Add cross conversion rules for conversions from boolean to the various other types (for setObject() with type argument). M java/client/org/apache/derby/client/am/DatabaseMetaData.java Add new method that tells whether the server understands boolean parameter value (that is, if it has the above mentioned fix in DRDAConnThread.java). Derbyall, suites.All and the client/server compatibility tests ran cleanly with the patch.
          Knut Anders Hatlen made changes -
          Attachment derby-4965-1b.diff [ 12468586 ]
          Hide
          Knut Anders Hatlen added a comment -

          The 1c patch adds more test cases to BooleanValuesTest.

          Show
          Knut Anders Hatlen added a comment - The 1c patch adds more test cases to BooleanValuesTest.
          Knut Anders Hatlen made changes -
          Attachment derby-4965-1c.diff [ 12468589 ]
          Knut Anders Hatlen made changes -
          Issue & fix info [Repro attached] [Patch Available, Repro attached]
          Hide
          Dag H. Wanvik added a comment -

          Patch looks good to me. I tried some simple testing of prepared statement in ij and the change in behavior was as expected.
          I think this change requires a release note.

          +1

          Show
          Dag H. Wanvik added a comment - Patch looks good to me. I tried some simple testing of prepared statement in ij and the change in behavior was as expected. I think this change requires a release note. +1
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for looking at the patch, Dag. Committed revision 1062743.

          I agree that a release note would be in order, so I've checked the Release Note Needed check box and will keep the issue open until one has been attached.

          I forgot to mention one change I made in the patch. I changed a comment in CrossConverters.java as shown below:

          // Convert from boolean source to target type.
          // In support of PS.setBoolean().

          • // See differences.html for DNC setBoolean() semantics.
            final Object setObject(int targetType, boolean source) throws SqlException {

          I didn't find any document named differences.html, nor any other document mentioning DNC setBoolean() semantics, so I removed that reference. Does anyone know which document the comment is talking about? We may need to update the document if it exists.

          Show
          Knut Anders Hatlen added a comment - Thanks for looking at the patch, Dag. Committed revision 1062743. I agree that a release note would be in order, so I've checked the Release Note Needed check box and will keep the issue open until one has been attached. I forgot to mention one change I made in the patch. I changed a comment in CrossConverters.java as shown below: // Convert from boolean source to target type. // In support of PS.setBoolean(). // See differences.html for DNC setBoolean() semantics. final Object setObject(int targetType, boolean source) throws SqlException { I didn't find any document named differences.html, nor any other document mentioning DNC setBoolean() semantics, so I removed that reference. Does anyone know which document the comment is talking about? We may need to update the document if it exists.
          Knut Anders Hatlen made changes -
          Fix Version/s 10.8.0.0 [ 12315561 ]
          Issue & fix info [Repro attached, Patch Available] [Release Note Needed, Repro attached]
          Hide
          Dag H. Wanvik added a comment -

          Yes, I noticed those two comments as well, I just presumed you had removed if for that reason.. no, I have no idea what it refers to.
          "DNC", I see we have a file tools/jar/dnc.properties. Looking inside, we see "Apache Derby Network Client", so it refers to what we no call the client driver, I guess. The file "differences.html" presumably described the semantics of the client driver relative to embedded as first donated to Apache? Maybe it's attached to a JIRA somewhere?

          Show
          Dag H. Wanvik added a comment - Yes, I noticed those two comments as well, I just presumed you had removed if for that reason.. no, I have no idea what it refers to. "DNC", I see we have a file tools/jar/dnc.properties. Looking inside, we see "Apache Derby Network Client", so it refers to what we no call the client driver, I guess. The file "differences.html" presumably described the semantics of the client driver relative to embedded as first donated to Apache? Maybe it's attached to a JIRA somewhere?
          Hide
          Myrna van Lunteren added a comment -

          I did some searching through version control history of when this code was developed (6 years ago) before getting contributed to apache, but even there there is no differences.html checked in, even though the earliest version of this file mentions it. No way to figure out what was in it now...+1 to removal of the reference.
          There were actually 2 references to that file in the CrossConverters class; we should probably remove the second reference too.

          Show
          Myrna van Lunteren added a comment - I did some searching through version control history of when this code was developed (6 years ago) before getting contributed to apache, but even there there is no differences.html checked in, even though the earliest version of this file mentions it. No way to figure out what was in it now...+1 to removal of the reference. There were actually 2 references to that file in the CrossConverters class; we should probably remove the second reference too.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for digging through ancient history, Myrna. I'm wondering if it
          could be referring to some version of this document:
          http://publib.boulder.ibm.com/infocenter/db2luw/v8/index.jsp?topic=/com.ibm.db2.udb.doc/ad/rjvjcdif.htm

          It says the following about the conversion changed in this issue:

          ,----

          Result of using setObject with a Boolean input type and a CHAR target type
          With the DB2 Universal JDBC Driver, when you execute
          PreparedStatement.setObject(parameterIndex,x,CHAR), and x is Boolean,
          the value "0" or "1" is inserted into the table column.
          With the DB2 JDBC Type 2 Driver, the string "false" or "true" is
          inserted into the table column. The table column length must be at
          least 5.
          `----

          Before the change, our drivers behaved like what's described for the
          DB2 Universal JDBC Driver. After the change, our drivers behave like
          what the document describes for the DB2 JDBC Type 2 Driver, with the
          exception that the value "true" can be inserted into a column whose
          length is 4.

          The other reference to differences.html in CrossConverters.java is
          about getBoolean() semantics. Here's what the document says about that
          case:

          ,----

          Result of using getBoolean to retrieve a value from a CHAR column
          With the DB2 Universal JDBC Driver, when you execute
          ResultSet.getBoolean or CallableStatement.getBoolean to retrieve a
          Boolean value from a CHAR column, and the column contains the value
          "false" or "0", the value false is returned. If the column contains
          any other value, true is returned.
          With the DB2 JDBC Type 2 Driver, when you execute ResultSet.getBoolean
          or CallableStatement.getBoolean to retrieve a Boolean value from a
          CHAR column, and the column contains the value "true" or "1", the
          value true is returned. If the column contains any other value, false
          is returned.
          `----

          Here, both our drivers behave like the DB2 Universal JDBC Driver. This
          behaviour is unchanged from previous releases.

          In any case, I think it's fine to remove the references. Will post a
          patch shortly.

          Show
          Knut Anders Hatlen added a comment - Thanks for digging through ancient history, Myrna. I'm wondering if it could be referring to some version of this document: http://publib.boulder.ibm.com/infocenter/db2luw/v8/index.jsp?topic=/com.ibm.db2.udb.doc/ad/rjvjcdif.htm It says the following about the conversion changed in this issue: ,---- Result of using setObject with a Boolean input type and a CHAR target type With the DB2 Universal JDBC Driver, when you execute PreparedStatement.setObject(parameterIndex,x,CHAR), and x is Boolean, the value "0" or "1" is inserted into the table column. With the DB2 JDBC Type 2 Driver, the string "false" or "true" is inserted into the table column. The table column length must be at least 5. `---- Before the change, our drivers behaved like what's described for the DB2 Universal JDBC Driver. After the change, our drivers behave like what the document describes for the DB2 JDBC Type 2 Driver, with the exception that the value "true" can be inserted into a column whose length is 4. The other reference to differences.html in CrossConverters.java is about getBoolean() semantics. Here's what the document says about that case: ,---- Result of using getBoolean to retrieve a value from a CHAR column With the DB2 Universal JDBC Driver, when you execute ResultSet.getBoolean or CallableStatement.getBoolean to retrieve a Boolean value from a CHAR column, and the column contains the value "false" or "0", the value false is returned. If the column contains any other value, true is returned. With the DB2 JDBC Type 2 Driver, when you execute ResultSet.getBoolean or CallableStatement.getBoolean to retrieve a Boolean value from a CHAR column, and the column contains the value "true" or "1", the value true is returned. If the column contains any other value, false is returned. `---- Here, both our drivers behave like the DB2 Universal JDBC Driver. This behaviour is unchanged from previous releases. In any case, I think it's fine to remove the references. Will post a patch shortly.
          Hide
          Knut Anders Hatlen added a comment -

          Here's a patch that removes the reference to differences.html in CrossConverters.java and adds some more details to the comments. The patch also updates the comments in the corresponding code on the embedded side. Additionally, the patch makes a trivial change to CrossConverters.getBooleanFromString() to save one unnecessary call to String.trim().

          Show
          Knut Anders Hatlen added a comment - Here's a patch that removes the reference to differences.html in CrossConverters.java and adds some more details to the comments. The patch also updates the comments in the corresponding code on the embedded side. Additionally, the patch makes a trivial change to CrossConverters.getBooleanFromString() to save one unnecessary call to String.trim().
          Knut Anders Hatlen made changes -
          Attachment update-comments.diff [ 12469399 ]
          Hide
          Knut Anders Hatlen added a comment -

          Revision: 1063656
          Author: kahatlen
          Date: 2011-01-26 10:32
          Message: DERBY-4965: Boolean to char conversion results in integer

          • Remove reference to non-existent document and improve comments
          • Reduce number of calls to String.trim() in getBooleanFromString()
          Show
          Knut Anders Hatlen added a comment - Revision: 1063656 Author: kahatlen Date: 2011-01-26 10:32 Message: DERBY-4965 : Boolean to char conversion results in integer Remove reference to non-existent document and improve comments Reduce number of calls to String.trim() in getBooleanFromString()
          Knut Anders Hatlen made changes -
          Original Estimate 0h [ 0 ]
          Remaining Estimate 0h [ 0 ]
          Hide
          Dag H. Wanvik added a comment -

          Looks good to me, +1. One question, you added a comment implying that JCC is still supported with Derby. I thought that was no longer the case? I can no longer find the mail I thought I saw fly by on this though..

          Show
          Dag H. Wanvik added a comment - Looks good to me, +1. One question, you added a comment implying that JCC is still supported with Derby. I thought that was no longer the case? I can no longer find the mail I thought I saw fly by on this though..
          Hide
          Myrna van Lunteren added a comment -

          Good eyes. You're right, there's no support for the combination of derby with JCC now.

          Show
          Myrna van Lunteren added a comment - Good eyes. You're right, there's no support for the combination of derby with JCC now.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for looking at the patch. I modified some comments on the embedded side that said the behaviour matched JCC, and added a comment saying the same on the client side. I didn't intend to imply that JCC is supported, just have some historical background as to why this behaviour is as it is. The behaviour of getBoolean() (return true for "False" for example) is a bit odd, so I felt an explanation was warranted.

          Show
          Knut Anders Hatlen added a comment - Thanks for looking at the patch. I modified some comments on the embedded side that said the behaviour matched JCC, and added a comment saying the same on the client side. I didn't intend to imply that JCC is supported, just have some historical background as to why this behaviour is as it is. The behaviour of getBoolean() (return true for "False" for example) is a bit odd, so I felt an explanation was warranted.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a release note for this issue.

          Show
          Knut Anders Hatlen added a comment - Attaching a release note for this issue.
          Knut Anders Hatlen made changes -
          Attachment releaseNote.html [ 12471922 ]
          Hide
          Knut Anders Hatlen added a comment -

          Resolving the issue so that it will show up in the release notes.

          Show
          Knut Anders Hatlen added a comment - Resolving the issue so that it will show up in the release notes.
          Knut Anders Hatlen made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Rick Hillegas added a comment -

          Adjusted release note.

          Show
          Rick Hillegas added a comment - Adjusted release note.
          Rick Hillegas made changes -
          Attachment releaseNote.html [ 12472330 ]
          Hide
          Rick Hillegas added a comment -

          Attaching a new version of the release note. This version does not have the blockquotes which violate the WCAG accessibility guidelines at http://www.w3.org/TR/WCAG10/

          Show
          Rick Hillegas added a comment - Attaching a new version of the release note. This version does not have the blockquotes which violate the WCAG accessibility guidelines at http://www.w3.org/TR/WCAG10/
          Rick Hillegas made changes -
          Attachment releaseNote.html [ 12475812 ]
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.1 [ 12316356 ]
          Fix Version/s 10.8.1.0 [ 12315561 ]
          Rick Hillegas made changes -
          Fix Version/s 10.8.1.2 [ 12316362 ]
          Fix Version/s 10.8.1.1 [ 12316356 ]
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Knut Anders Hatlen made changes -
          Link This issue is related to DERBY-5788 [ DERBY-5788 ]
          Gavin made changes -
          Workflow jira [ 12542133 ] Default workflow, editable Closed status [ 12801135 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          2h 32m 1 Knut Anders Hatlen 11/Jan/11 13:01
          In Progress In Progress Resolved Resolved
          47d 23h 40m 1 Knut Anders Hatlen 28/Feb/11 12:42
          Resolved Resolved Closed Closed
          121d 20h 39m 1 Knut Anders Hatlen 30/Jun/11 10:21

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development