Derby
  1. Derby
  2. DERBY-853

ResultSetMetaData.getScale returns inconsistent values for DOUBLE type.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 10.2.1.6
    • Fix Version/s: 10.13.1.0
    • Component/s: JDBC
    • Urgency:
      Normal
    • Issue & fix info:
      Newcomer, Repro attached

      Description

      If a DOUBLE column is returned in the result set then getScale() returns 0.
      If a DOUBLE expression is returned and the expression is the result of a DOUBLE combined with a DECIMAL then it seems the scale from the decimal sometimes affects the result set metadata.

      E.g. DECIMAL(10,2) - DOUBLE returns a DOUBLE with getScale() returning 2.

      See the test output for jdbcapi/metadata.java

      double – precision: 15 scale: 0 display size: 22 type name: DOUBLE
      double precision - dec(10,2) – precision: 15 scale: 0 display size: 22 type name: DOUBLE
      dec(10,2) - double precision – precision: 15 scale: 2 display size: 22 type name: DOUBLE

      First line is a DOUBLE column, second is DOUBLE - DECIMAL, third is DECIMAL - DOUBLE

      I assume the scale should always be zero for a DOUBLE, as it holds no meaning, but I can't see any proof of that in JDBC spec, javadoc or tutorial book.

      1. BryanPossibleIdea.diff
        2 kB
        Bryan Pendleton
      2. Derby-853_2.diff
        2 kB
        Danoja Dias
      3. Derby-853_3.diff
        3 kB
        Danoja Dias
      4. Derby-853.diff
        0.6 kB
        Danoja Dias
      5. Derby853.java
        0.7 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Rick Hillegas added a comment -

          Triaged for 10.5.3: assigne normal urgency.

          Show
          Rick Hillegas added a comment - Triaged for 10.5.3: assigne normal urgency.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching Derby853.java which reproduces the problem. It subtracts a DOUBLE from a DECIMAL(10,2) and prints the scale and the type name of the result. On trunk, it prints:

          scale: 2
          type: DOUBLE

          Show
          Knut Anders Hatlen added a comment - Attaching Derby853.java which reproduces the problem. It subtracts a DOUBLE from a DECIMAL(10,2) and prints the scale and the type name of the result. On trunk, it prints: scale: 2 type: DOUBLE
          Hide
          Danoja Dias added a comment -

          I think the problem happens in getScale() method in NumericTypeCompiler.java .

          In this method, getStoredFormatIdFromTypeId() is not equal to Decimal it returns
          the scale value of leftType.

          Show
          Danoja Dias added a comment - I think the problem happens in getScale() method in NumericTypeCompiler.java . In this method, getStoredFormatIdFromTypeId() is not equal to Decimal it returns the scale value of leftType.
          Hide
          Danoja Dias added a comment -

          I attached a patch. Could you please give me any comments about this patch.

          thanks.

          Show
          Danoja Dias added a comment - I attached a patch. Could you please give me any comments about this patch. thanks.
          Hide
          Bryan Pendleton added a comment -

          It seems like a good approach – did it fix the Derby853.java test?

          I think you are right that the problem is that we are using 'leftType',
          which isn't correct in this case, because the type precedence rules
          mean that at line 258 we have noticed that combining a DECIMAL and a
          DOUBLE means we have a result of DOUBLE.

          I wonder if there is a way to fix this somewhere near line 258 instead,
          because it seems like at line 258 we shouldn't even be calling the
          getScale() method if higherType is not a DECIMAL type, so maybe that
          call get getScale() should be inside the "if" test at line 260?

          bryan

          Show
          Bryan Pendleton added a comment - It seems like a good approach – did it fix the Derby853.java test? I think you are right that the problem is that we are using 'leftType', which isn't correct in this case, because the type precedence rules mean that at line 258 we have noticed that combining a DECIMAL and a DOUBLE means we have a result of DOUBLE. I wonder if there is a way to fix this somewhere near line 258 instead, because it seems like at line 258 we shouldn't even be calling the getScale() method if higherType is not a DECIMAL type, so maybe that call get getScale() should be inside the "if" test at line 260? bryan
          Hide
          Danoja Dias added a comment -

          did it fix the Derby853.java test?

          Yes. It fixed Derby853.java test.

          I wonder if there is a way to fix this somewhere near line 258 instead,
          because it seems like at line 258 we shouldn't even be calling the
          getScale() method if higherType is not a DECIMAL type, so maybe that
          call get getScale() should be inside the "if" test at line 260?

          Yes. We shouldn't call getScale() is it is not decimal.
          I'll check it and attach a new patch.

          Show
          Danoja Dias added a comment - did it fix the Derby853.java test? Yes. It fixed Derby853.java test. I wonder if there is a way to fix this somewhere near line 258 instead, because it seems like at line 258 we shouldn't even be calling the getScale() method if higherType is not a DECIMAL type, so maybe that call get getScale() should be inside the "if" test at line 260? Yes. We shouldn't call getScale() is it is not decimal. I'll check it and attach a new patch.
          Hide
          Danoja Dias added a comment -

          I attached doing changes. This patch also passes the test in Derby853.java

          Show
          Danoja Dias added a comment - I attached doing changes. This patch also passes the test in Derby853.java
          Hide
          Danoja Dias added a comment -

          All tests are clean with this patch

          Show
          Danoja Dias added a comment - All tests are clean with this patch
          Hide
          Bryan Pendleton added a comment -

          Which test suite do you think would be a good place to add the test
          cases from the Derby853.java code?

          Show
          Bryan Pendleton added a comment - Which test suite do you think would be a good place to add the test cases from the Derby853.java code?
          Hide
          Danoja Dias added a comment -

          I think this suits for
          java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ResultSetMiscTest.java

          I'll add it with a new method.

          Show
          Danoja Dias added a comment - I think this suits for java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ResultSetMiscTest.java I'll add it with a new method.
          Hide
          Danoja Dias added a comment -

          This patch adds the tests to test this bug.

          This patch touches
          M java/engine/org/apache/derby/impl/sql/compile/NumericTypeCompiler.java
          M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ResultSetMiscTest.java

          Show
          Danoja Dias added a comment - This patch adds the tests to test this bug. This patch touches M java/engine/org/apache/derby/impl/sql/compile/NumericTypeCompiler.java M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ResultSetMiscTest.java
          Hide
          Bryan Pendleton added a comment -

          Hi Danoja,

          I was wondering if you could look at a slight variation on your patch,
          which I have attached as "BryanPossibleIdea.diff".

          It is very similar to your patch, with two differences:

          1) If the higher-precedence type isn't DECIMAL, I changed it to
          simply set the scale to 0. I don't think it matters terribly much whether
          the alternate type is DOUBLE, or BIGINT, or any other type; if it's
          not DECIMAL, the scale should be 0.

          2) I also added a test of getPrecision to the test case that you
          added to confirm that, whichever order the arguments are in
          the test case, the precision still comes back 15. I didn't discover
          any new problem; I just added the additional assertion on getPrecision

          What do you think about this idea?

          For me, it passes your new test case, and it also passes all
          the existing test suites.

          thanks,

          bryan

          Show
          Bryan Pendleton added a comment - Hi Danoja, I was wondering if you could look at a slight variation on your patch, which I have attached as "BryanPossibleIdea.diff". It is very similar to your patch, with two differences: 1) If the higher-precedence type isn't DECIMAL, I changed it to simply set the scale to 0. I don't think it matters terribly much whether the alternate type is DOUBLE, or BIGINT, or any other type; if it's not DECIMAL, the scale should be 0. 2) I also added a test of getPrecision to the test case that you added to confirm that, whichever order the arguments are in the test case, the precision still comes back 15. I didn't discover any new problem; I just added the additional assertion on getPrecision What do you think about this idea? For me, it passes your new test case, and it also passes all the existing test suites. thanks, bryan
          Hide
          Danoja Dias added a comment -

          Hi Bryan,

          I think since scale of all other types(that are supported for operations like "-") should be 0, this is a simple and good approach.

          thanks.

          Show
          Danoja Dias added a comment - Hi Bryan, I think since scale of all other types(that are supported for operations like "-") should be 0, this is a simple and good approach. thanks.
          Hide
          Danoja Dias added a comment -

          All test are clean in my system too.

          Show
          Danoja Dias added a comment - All test are clean in my system too.
          Hide
          ASF subversion and git services added a comment -

          Commit 1755133 from Bryan Pendleton in branch 'code/trunk'
          [ https://svn.apache.org/r1755133 ]

          DERBY-853: ResultSetMetaData.getScale returns inconsistent values

          This patch was contributed by Danoja Dias (danojadias at gmail dot com)

          When a SQL statement contains arithmetic expressions, the result of the
          expression may be of a different type than the operands to the expression,
          due to type precedence rules which may require promoting the operand
          values during evaluation of the expression.

          For example, subtracting a DOUBLE from a DECIMAL results in a DOUBLE.

          In some of these cases, Derby was reporting that the result column
          had a non-zero scale, although the result column was not of DECIMAL type.

          This change modifies the NumericTypeCompiler so that it only computes
          a non-zero scale for the result column when it is of DECIMAL type.

          Show
          ASF subversion and git services added a comment - Commit 1755133 from Bryan Pendleton in branch 'code/trunk' [ https://svn.apache.org/r1755133 ] DERBY-853 : ResultSetMetaData.getScale returns inconsistent values This patch was contributed by Danoja Dias (danojadias at gmail dot com) When a SQL statement contains arithmetic expressions, the result of the expression may be of a different type than the operands to the expression, due to type precedence rules which may require promoting the operand values during evaluation of the expression. For example, subtracting a DOUBLE from a DECIMAL results in a DOUBLE. In some of these cases, Derby was reporting that the result column had a non-zero scale, although the result column was not of DECIMAL type. This change modifies the NumericTypeCompiler so that it only computes a non-zero scale for the result column when it is of DECIMAL type.
          Hide
          Danoja Dias added a comment -

          I think, we can resolve this issue.

          Show
          Danoja Dias added a comment - I think, we can resolve this issue.
          Hide
          Bryan Pendleton added a comment -

          We've completed all the planned work for this issue. Marking fixed.

          Show
          Bryan Pendleton added a comment - We've completed all the planned work for this issue. Marking fixed.

            People

            • Assignee:
              Danoja Dias
              Reporter:
              Daniel John Debrunner
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development