Derby
  1. Derby
  2. DERBY-4658

Allow explicit casts of string values to BOOLEAN

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.7.1.1
    • Component/s: SQL
    • Labels:
      None

      Description

      The SQL Standard allows strings to be explicitly cast to BOOLEAN values. Strings are the only type (other than BOOLEAN itself) which can be cast to BOOLEAN. As part of our expanding support for the BOOLEAN datatype, we should allow these casts.

      Casting string types to boolean is defined by part 2, section 6.12 (<cast specification>), general rule 20:

      a) Trim whitespace off the string

      b) Then apply the rules in section 5.3 (<literal>). This means that the trimmed string must be 'TRUE', 'FALSE', or 'UNKNOWN', regardless of case.

      c) Otherwise, raise an exception.

      1. derby-4658-01-aa-booleanCasts.diff
        13 kB
        Rick Hillegas
      2. derby-4658-01-ab-booleanCasts.diff
        15 kB
        Rick Hillegas
      3. nullability.diff
        3 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Rick Hillegas added a comment -

          Thanks, Knut.

          Show
          Rick Hillegas added a comment - Thanks, Knut.
          Hide
          Knut Anders Hatlen added a comment -

          All the regression tests ran cleanly. Committed revision 955940.

          Show
          Knut Anders Hatlen added a comment - All the regression tests ran cleanly. Committed revision 955940.
          Hide
          Knut Anders Hatlen added a comment -

          I followed the calls from ConstantNode.init(), and it does seem like the data type of the constant is initialized with the nullability specified in the second argument. I tried to apply the first patch and remove the call to setNullability(), and then BooleanValuesTest failed in test_08_stringCasts, but only in client/server mode. When I applied the second patch, BooleanValuesTest ran cleanly also with setNullability() removed.

          The attached patch nullability.diff adds a new test case that tests the nullability explicitly. It fails (also in embedded mode) when setNullability() is removed from the first patch, and runs cleanly when setNullability() is removed from the second patch. The patch also removes the call to setNullability() from CastNode.

          Show
          Knut Anders Hatlen added a comment - I followed the calls from ConstantNode.init(), and it does seem like the data type of the constant is initialized with the nullability specified in the second argument. I tried to apply the first patch and remove the call to setNullability(), and then BooleanValuesTest failed in test_08_stringCasts, but only in client/server mode. When I applied the second patch, BooleanValuesTest ran cleanly also with setNullability() removed. The attached patch nullability.diff adds a new test case that tests the nullability explicitly. It fails (also in embedded mode) when setNullability() is removed from the first patch, and runs cleanly when setNullability() is removed from the second patch. The patch also removes the call to setNullability() from CastNode.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me. Committed derby-4658-01-ab-booleanCasts.diff at revision 955666.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me. Committed derby-4658-01-ab-booleanCasts.diff at revision 955666.
          Hide
          Rick Hillegas added a comment -

          On second thought, Knut, you may be right. You are welcome to sand this down further after I commit this patch.

          Show
          Rick Hillegas added a comment - On second thought, Knut, you may be right. You are welcome to sand this down further after I commit this patch.
          Hide
          Rick Hillegas added a comment -

          Hi Knut. Thanks for taking a look at the new patch. I don't think that the changes in BooleanConstantNode affect the line in CastNode which you're referring to. The problem there is that the nullability of the CastNode is copied from the nullability of the source cast operand. In the following statement

          cast ( 'unknown' as boolean )

          the source operand is a string constant which is not nullable, but the result of the cast is nullable--in fact, it is NULL.

          Show
          Rick Hillegas added a comment - Hi Knut. Thanks for taking a look at the new patch. I don't think that the changes in BooleanConstantNode affect the line in CastNode which you're referring to. The problem there is that the nullability of the CastNode is copied from the nullability of the source cast operand. In the following statement cast ( 'unknown' as boolean ) the source operand is a string constant which is not nullable, but the result of the cast is nullable--in fact, it is NULL.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Rick. I see that CastNode still calls cn.setNullability(true). Weren't the changes in BooleanConstantNode.init() sufficient to get the nullability correctly initialized?

          Show
          Knut Anders Hatlen added a comment - Thanks, Rick. I see that CastNode still calls cn.setNullability(true). Weren't the changes in BooleanConstantNode.init() sufficient to get the nullability correctly initialized?
          Hide
          Rick Hillegas added a comment -

          Thanks for the quick review, Knut. I have incorporated your suggestions in the next version of this patch, which I am attaching: derby-4658-01-ab-booleanCasts.diff. I plan to commit this revised patch if the tests pass.

          In the LangScripts test there was a diff, which this new patch addresses too:

          M java/testing/org/apache/derbyTesting/functionTests/master/cast.out

          An error message changed because casts to BOOLEAN now pass the parser.

          Show
          Rick Hillegas added a comment - Thanks for the quick review, Knut. I have incorporated your suggestions in the next version of this patch, which I am attaching: derby-4658-01-ab-booleanCasts.diff. I plan to commit this revised patch if the tests pass. In the LangScripts test there was a diff, which this new patch addresses too: M java/testing/org/apache/derbyTesting/functionTests/master/cast.out An error message changed because casts to BOOLEAN now pass the parser.
          Hide
          Knut Anders Hatlen added a comment -

          The patch looks fine to me. Some minor comments, though:

          In BooleanConstantNode.init(), would it be better to pass Boolean.TRUE as the second argument to super.init()? That's the argument that specifies nullability. Perhaps that would also remove the need to call setNullability(true) in CastNode?

          The tests look fairly complete, but they all use true/false/unknown in lowercase. Would it be worthwhile to add a test case to verify that strings with upper case or mixed case also can be cast to boolean?

          And, a nit, this change in BooleanConstantNode.init() is unnecessary since (arg1 instanceof Boolean) implies (arg1 != null):

          • if (arg1 instanceof Boolean)
            + if ( (arg1 != null) && (arg1 instanceof Boolean) )

          Perhaps, for clarity, it may be better to have the null case first, though:

          if (arg1 == null)

          { ... } else if (arg1 instanceof Boolean) { ... }

          else

          { ... }
          Show
          Knut Anders Hatlen added a comment - The patch looks fine to me. Some minor comments, though: In BooleanConstantNode.init(), would it be better to pass Boolean.TRUE as the second argument to super.init()? That's the argument that specifies nullability. Perhaps that would also remove the need to call setNullability(true) in CastNode? The tests look fairly complete, but they all use true/false/unknown in lowercase. Would it be worthwhile to add a test case to verify that strings with upper case or mixed case also can be cast to boolean? And, a nit, this change in BooleanConstantNode.init() is unnecessary since (arg1 instanceof Boolean) implies (arg1 != null): if (arg1 instanceof Boolean) + if ( (arg1 != null) && (arg1 instanceof Boolean) ) Perhaps, for clarity, it may be better to have the null case first, though: if (arg1 == null) { ... } else if (arg1 instanceof Boolean) { ... } else { ... }
          Hide
          Rick Hillegas added a comment -

          Attaching derby-4658-01-aa-booleanCasts.diff. This patch makes it possible to cast values to BOOLEAN. I need to run regression tests.

          Makes the following changes:

          1) Makes parser accept CAST (... AS BOOLEAN)

          2) Allows those casts to succeed provided that the source datatype is a string and the actual value satisfies the SQL Standard's rules.

          3) Adds regression tests for this feature.

          Touches the following files:

          --------------

          M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj

          Parser now accepts explicit casts to BOOLEAN.

          --------------

          M java/engine/org/apache/derby/impl/sql/compile/CastNode.java
          M java/engine/org/apache/derby/impl/sql/compile/BooleanConstantNode.java
          M java/engine/org/apache/derby/iapi/types/SQLBoolean.java

          Standard support for the string value "UNKNOWN".

          --------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/lang/BooleanValuesTest.java

          Regression tests.

          Show
          Rick Hillegas added a comment - Attaching derby-4658-01-aa-booleanCasts.diff. This patch makes it possible to cast values to BOOLEAN. I need to run regression tests. Makes the following changes: 1) Makes parser accept CAST (... AS BOOLEAN) 2) Allows those casts to succeed provided that the source datatype is a string and the actual value satisfies the SQL Standard's rules. 3) Adds regression tests for this feature. Touches the following files: -------------- M java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj Parser now accepts explicit casts to BOOLEAN. -------------- M java/engine/org/apache/derby/impl/sql/compile/CastNode.java M java/engine/org/apache/derby/impl/sql/compile/BooleanConstantNode.java M java/engine/org/apache/derby/iapi/types/SQLBoolean.java Standard support for the string value "UNKNOWN". -------------- M java/testing/org/apache/derbyTesting/functionTests/tests/lang/BooleanValuesTest.java Regression tests.

            People

            • Assignee:
              Rick Hillegas
              Reporter:
              Rick Hillegas
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development