Derby
  1. Derby
  2. DERBY-4698

Simple query with HAVING clause crashes with NullPointerException

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.3.1.4, 10.3.2.1, 10.3.3.0, 10.4.1.3, 10.4.2.0, 10.5.1.1, 10.5.2.0, 10.5.3.0, 10.6.1.0
    • Fix Version/s: 10.5.3.1, 10.6.2.1, 10.7.1.1
    • Component/s: SQL
    • Labels:
      None
    • Environment:
      Windows, Java 1.6
    • Bug behavior facts:
      Crash, Seen in production, Wrong query result

      Description

      Running a simple SQL query containing a having clause causes a NullPointerException. I originally encountered this in 10.4.2.0, but have also reproduces this in 10.5.3.0 and the latest 10.6.1.0. I raised this on the mailing list too, and Knut said it also fails on trunk - see http://thread.gmane.org/gmane.comp.apache.db.derby.user/12782

      The query (created throw Hibernate) that causes the crash is:

      SELECT user0_.user_id AS col_0_0_,
      SUM(account2_.balance) AS col_1_0_
      FROM tbl_user user0_
      INNER JOIN tbl_user_account accountlin1_
      ON user0_.user_id = accountlin1_.user_id
      INNER JOIN tbl_account account2_
      ON accountlin1_.account_id = account2_.account_id
      WHERE user0_.deleted = 'N'
      AND ( account2_.account_type IN ( 'USER-01', 'USER' ) )
      GROUP BY user0_.user_id
      HAVING SUM(account2_.balance) >= 100.0

      However I simplified it to the following and still caused a crash (though in 10.4.2.0 I found that without the "where" clause is didn't crash but returned no results when it should have).

      SELECT u.user_id,
      SUM(a.balance)
      FROM tbl_user u
      INNER JOIN tbl_user_account al
      ON u.user_id = al.user_id
      INNER JOIN tbl_account a
      ON al.account_id = a.account_id
      GROUP BY u.user_id
      HAVING sum(a.balance) >= 10.0

      The derby log showed the following stace trace for 10.6.1.0:
      2010-06-14 04:59:24.942 GMT Thread[main,5,main] (XID = 5824013), (SESSIONID = 1), (DATABASE = C:\Development\pc-ng-branch\server\working\data\internal/derby), (DRDAID = null), Failed Statement is: SELECT u.user_id user_id,
      SUM(a.balance) acct_sum
      FROM tbl_user u
      INNER JOIN tbl_user_account al
      ON u.user_id = al.user_id
      INNER JOIN tbl_account a
      ON al.account_id = a.account_id
      GROUP BY u.user_id
      HAVING sum(a.balance) >= 1.0
      java.lang.NullPointerException
      at org.apache.derby.impl.sql.compile.ColumnReference.remapColumnReferencesToExpressions(Unknown Source)
      at org.apache.derby.impl.sql.compile.AggregateNode.getNewExpressionResultColumn(Unknown Source)
      at org.apache.derby.impl.sql.compile.GroupByNode.addAggregateColumns(Unknown Source)
      at org.apache.derby.impl.sql.compile.GroupByNode.addNewColumnsForAggregation(Unknown Source)
      at org.apache.derby.impl.sql.compile.GroupByNode.addAggregates(Unknown Source)
      at org.apache.derby.impl.sql.compile.GroupByNode.init(Unknown Source)
      at org.apache.derby.iapi.sql.compile.NodeFactory.getNode(Unknown Source)
      at org.apache.derby.impl.sql.compile.SelectNode.genProjectRestrict(Unknown Source)
      at org.apache.derby.impl.sql.compile.SelectNode.modifyAccessPaths(Unknown Source)
      at org.apache.derby.impl.sql.compile.DMLStatementNode.optimizeStatement(Unknown Source)
      at org.apache.derby.impl.sql.compile.CursorNode.optimizeStatement(Unknown Source)
      at org.apache.derby.impl.sql.GenericStatement.prepMinion(Unknown Source)
      at org.apache.derby.impl.sql.GenericStatement.prepare(Unknown Source)
      at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(Unknown Source)
      at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)
      at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)
      at org.apache.derby.impl.tools.ij.ij.executeImmediate(Unknown Source)
      at org.apache.derby.impl.tools.ij.utilMain.doCatch(Unknown Source)
      at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(Unknown Source)
      at org.apache.derby.impl.tools.ij.utilMain.go(Unknown Source)
      at org.apache.derby.impl.tools.ij.Main.go(Unknown Source)
      at org.apache.derby.impl.tools.ij.Main.mainCore(Unknown Source)
      at org.apache.derby.impl.tools.ij.Main.main(Unknown Source)
      at org.apache.derby.tools.ij.main(Unknown Source)
      Cleanup action completed

      1. derby-crash-10.6.1.0.log
        3 kB
        Matt Doran
      2. derby-crash-10.5.3.0.log
        3 kB
        Matt Doran
      3. derby-crash-10.4.2.0.log
        3 kB
        Matt Doran
      4. derby.zip
        615 kB
        Matt Doran
      5. derby-4698-1.diff
        6 kB
        Dag H. Wanvik
      6. derby-4698-1.stat
        0.4 kB
        Dag H. Wanvik
      7. derby-4698-2.diff
        11 kB
        Dag H. Wanvik
      8. derby-4698-2.stat
        0.5 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.
          Hide
          Knut Anders Hatlen added a comment -

          Changed 10.5 fix version to match the version on the branch. Resolving the issue.

          Show
          Knut Anders Hatlen added a comment - Changed 10.5 fix version to match the version on the branch. Resolving the issue.
          Hide
          Kathey Marsden added a comment -

          reopen for back port to 10.5

          Show
          Kathey Marsden added a comment - reopen for back port to 10.5
          Hide
          Mike Matrigali added a comment -

          backported fix to 10.5 branch.

          s105_ibm16:8>svn commit

          Sending java\engine\org\apache\derby\impl\sql\compile\AggregateNode.java
          Sending java\engine\org\apache\derby\impl\sql\compile\FromList.java
          Sending java\engine\org\apache\derby\impl\sql\compile\FromSubquery.java
          Sending java\engine\org\apache\derby\impl\sql\compile\FromTable.java
          Sending java\engine\org\apache\derby\impl\sql\compile\JoinNode.java
          Sending java\engine\org\apache\derby\impl\sql\compile\SelectNode.java
          Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\GroupByTest.java
          Transmitting file data .......
          Committed revision 959697.

          Show
          Mike Matrigali added a comment - backported fix to 10.5 branch. s105_ibm16:8>svn commit Sending java\engine\org\apache\derby\impl\sql\compile\AggregateNode.java Sending java\engine\org\apache\derby\impl\sql\compile\FromList.java Sending java\engine\org\apache\derby\impl\sql\compile\FromSubquery.java Sending java\engine\org\apache\derby\impl\sql\compile\FromTable.java Sending java\engine\org\apache\derby\impl\sql\compile\JoinNode.java Sending java\engine\org\apache\derby\impl\sql\compile\SelectNode.java Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\GroupByTest.java Transmitting file data ....... Committed revision 959697.
          Hide
          Mike Matrigali added a comment -

          resetting original bug fixer.

          Show
          Mike Matrigali added a comment - resetting original bug fixer.
          Hide
          Mike Matrigali added a comment -

          I am looking at backporting this fix to 10.5.

          Show
          Mike Matrigali added a comment - I am looking at backporting this fix to 10.5.
          Hide
          Dag H. Wanvik added a comment -

          The NPE can be seen back to 10.3. In 10.2 the query doesn't compile.

          Show
          Dag H. Wanvik added a comment - The NPE can be seen back to 10.3. In 10.2 the query doesn't compile.
          Hide
          Dag H. Wanvik added a comment -

          Backported cleanly to the 10.6 branch as svn 956238, resolving.

          Show
          Dag H. Wanvik added a comment - Backported cleanly to the 10.6 branch as svn 956238, resolving.
          Hide
          Dag H. Wanvik added a comment -

          Committed to trunk as svn 956234.

          Show
          Dag H. Wanvik added a comment - Committed to trunk as svn 956234.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for fixing this and writing such a clear description of the problem, Dag. Your suggested approach looks good to me. I've verified that the new test case in GroupByTest fails without the fix and passes with the fix. +1

          Show
          Knut Anders Hatlen added a comment - Thanks for fixing this and writing such a clear description of the problem, Dag. Your suggested approach looks good to me. I've verified that the new test case in GroupByTest fails without the fix and passes with the fix. +1
          Hide
          Matt Doran added a comment -

          Yup, that looks right.

          Show
          Matt Doran added a comment - Yup, that looks right.
          Hide
          Dag H. Wanvik added a comment -

          Uploading rev 2 of this patch, which adds a repro fixture to GroupByTest. Regressions ran ok with the first revision.

          Matt, the results I see for the first query is

          COL_0_0_ |COL_1_0_
          -------------------------------------------
          1004 |260.7

          and for the second:
          COL_0_0_ |COL_1_0_
          -------------------------------------------
          1001 |10.0
          1003 |10.0
          1004 |260.7
          1005 |10.0
          1006 |10.0
          1007 |10.0
          1008 |10.0
          2002 |11.0
          3004 |12.5

          Do these look as you expect with the data in the zipped db?

          Show
          Dag H. Wanvik added a comment - Uploading rev 2 of this patch, which adds a repro fixture to GroupByTest. Regressions ran ok with the first revision. Matt, the results I see for the first query is COL_0_0_ |COL_1_0_ ------------------------------------------- 1004 |260.7 and for the second: COL_0_0_ |COL_1_0_ ------------------------------------------- 1001 |10.0 1003 |10.0 1004 |260.7 1005 |10.0 1006 |10.0 1007 |10.0 1008 |10.0 2002 |11.0 3004 |12.5 Do these look as you expect with the data in the zipped db?
          Hide
          Dag H. Wanvik added a comment - - edited

          Uploading a preliminary patch which seems to fix this problem.

          My theory is the following: As part of DERBY-3880 a fix was inserted to handle the case of column references in HAVING clauses being wrong after JOIN flattening.

          Normally, such column references are updated as part of the flattening operation in the preprocess phase; i.e. in the first of the three main phases in the statement optimization:

          {preprocess, optimize, modifyAccessPaths}

          cf DMLStatementNode#optimizeStatement. The flattening is driven by SelectNode#preprocess in the call to fromList.flattenFromTables.

          The fix for DERBY-3880 involved remapping the now wrong column reference in the HAVING clause (i.e. after join flattening) as part of the modifyAccessPaths phase, cf the fix in AggregateNode#getNewExpressionResultColumn, ca line 566 on trunk.

          At the time, Bryan asked if the fix wasn't somewhat odd, since the phase in which the column reference remapping for the having clause was now done, was different from the other column reference fixups and suggested that it might be more appropriate to remap column references from the HAVING clause by passing in the havingClause in the flattenFromTables call made by SelectNode.

          I agree with Bryan that this sounds like a sounder approach, and it is the approach taken by this patch. It makes both queries shown in this JIRA issue work with the supplied database.

          The problem encountered by the old fix in this case, is that the column reference's VCN's sourceResultSet (due to the presence of indexes?) has a result column which have already been converted to a CurrentRowLocationNode, cf the rewrite calls to addRCForRID that happen in fromBaseTable#changeAccessPath. This seems a correct transformation, but makes the remapping fail. So, essentially, the rewrite is attempted too late, at least for the problematic queries.

          On 10.6, with debug code enabled, we don't see a NPE, but an assert that the desired column (A.BALANCE) could not be found. On trunk, the assert is different due to the change introduced by DERBY-4679: it complains that it does not expect to see a CurrentRowLocationNode, alas. In both cases, the problem is the same, the CurrentRowLocationNode's presence is unexpected and makes the remapping fail.

          By treating column references in the havingClause uniformly with other column references at flattening time in the preprocess phase, there is no need for the old fix code, so the patch removes that.

          GroupByTest which contains the test case for DERBY-3880 still passes with the patch, so it would seem the patch is a valid alternate solution for DERBY-3880 also.

          Will run full regressions and try to make a simple repro so we can add a new test case for this issue.

          Show
          Dag H. Wanvik added a comment - - edited Uploading a preliminary patch which seems to fix this problem. My theory is the following: As part of DERBY-3880 a fix was inserted to handle the case of column references in HAVING clauses being wrong after JOIN flattening. Normally, such column references are updated as part of the flattening operation in the preprocess phase; i.e. in the first of the three main phases in the statement optimization: {preprocess, optimize, modifyAccessPaths} cf DMLStatementNode#optimizeStatement. The flattening is driven by SelectNode#preprocess in the call to fromList.flattenFromTables. The fix for DERBY-3880 involved remapping the now wrong column reference in the HAVING clause (i.e. after join flattening) as part of the modifyAccessPaths phase, cf the fix in AggregateNode#getNewExpressionResultColumn, ca line 566 on trunk. At the time, Bryan asked if the fix wasn't somewhat odd, since the phase in which the column reference remapping for the having clause was now done, was different from the other column reference fixups and suggested that it might be more appropriate to remap column references from the HAVING clause by passing in the havingClause in the flattenFromTables call made by SelectNode. I agree with Bryan that this sounds like a sounder approach, and it is the approach taken by this patch. It makes both queries shown in this JIRA issue work with the supplied database. The problem encountered by the old fix in this case, is that the column reference's VCN's sourceResultSet (due to the presence of indexes?) has a result column which have already been converted to a CurrentRowLocationNode, cf the rewrite calls to addRCForRID that happen in fromBaseTable#changeAccessPath. This seems a correct transformation, but makes the remapping fail. So, essentially, the rewrite is attempted too late, at least for the problematic queries. On 10.6, with debug code enabled, we don't see a NPE, but an assert that the desired column (A.BALANCE) could not be found. On trunk, the assert is different due to the change introduced by DERBY-4679 : it complains that it does not expect to see a CurrentRowLocationNode, alas. In both cases, the problem is the same, the CurrentRowLocationNode's presence is unexpected and makes the remapping fail. By treating column references in the havingClause uniformly with other column references at flattening time in the preprocess phase, there is no need for the old fix code, so the patch removes that. GroupByTest which contains the test case for DERBY-3880 still passes with the patch, so it would seem the patch is a valid alternate solution for DERBY-3880 also. Will run full regressions and try to make a simple repro so we can add a new test case for this issue.
          Hide
          Kathey Marsden added a comment -

          There was a pretty major rewrite of the HAVING logic in 10.3 DERBY-681, so that is the change I usually look to first to identify the source of HAVING regressions in 10.3. Unfortunately, because of the dramatic nature of the change, identifying it as the source of the problem doesn't usually lead to an obvious solution.

          Show
          Kathey Marsden added a comment - There was a pretty major rewrite of the HAVING logic in 10.3 DERBY-681 , so that is the change I usually look to first to identify the source of HAVING regressions in 10.3. Unfortunately, because of the dramatic nature of the change, identifying it as the source of the problem doesn't usually lead to an obvious solution.
          Hide
          Knut Anders Hatlen added a comment -

          The query triggers an assert in debug builds:

          org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED expected BaseColumnNode, found: class org.apache.derby.impl.sql.compile.CurrentRowLocationNode
          at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityManager.java:120)
          at org.apache.derby.impl.sql.compile.ResultColumnList.getResultColumn(ResultColumnList.java:373)
          at org.apache.derby.impl.sql.compile.ColumnReference.remapColumnReferencesToExpressions(ColumnReference.java:888)
          at org.apache.derby.impl.sql.compile.AggregateNode.getNewExpressionResultColumn(AggregateNode.java:568)
          at org.apache.derby.impl.sql.compile.GroupByNode.addAggregateColumns(GroupByNode.java:715)
          at org.apache.derby.impl.sql.compile.GroupByNode.addNewColumnsForAggregation(GroupByNode.java:558)
          at org.apache.derby.impl.sql.compile.GroupByNode.addAggregates(GroupByNode.java:245)
          at org.apache.derby.impl.sql.compile.GroupByNode.init(GroupByNode.java:184)
          at org.apache.derby.iapi.sql.compile.NodeFactory.getNode(NodeFactory.java:273)
          at org.apache.derby.impl.sql.compile.SelectNode.genProjectRestrict(SelectNode.java:1481)
          at org.apache.derby.impl.sql.compile.SelectNode.modifyAccessPaths(SelectNode.java:2121)
          at org.apache.derby.impl.sql.compile.DMLStatementNode.optimizeStatement(DMLStatementNode.java:317)
          at org.apache.derby.impl.sql.compile.CursorNode.optimizeStatement(CursorNode.java:558)
          at org.apache.derby.impl.sql.GenericStatement.prepMinion(GenericStatement.java:381)
          at org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java:90)
          at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(GenericLanguageConnectionContext.java:843)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:606)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555)

          Show
          Knut Anders Hatlen added a comment - The query triggers an assert in debug builds: org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED expected BaseColumnNode, found: class org.apache.derby.impl.sql.compile.CurrentRowLocationNode at org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityManager.java:120) at org.apache.derby.impl.sql.compile.ResultColumnList.getResultColumn(ResultColumnList.java:373) at org.apache.derby.impl.sql.compile.ColumnReference.remapColumnReferencesToExpressions(ColumnReference.java:888) at org.apache.derby.impl.sql.compile.AggregateNode.getNewExpressionResultColumn(AggregateNode.java:568) at org.apache.derby.impl.sql.compile.GroupByNode.addAggregateColumns(GroupByNode.java:715) at org.apache.derby.impl.sql.compile.GroupByNode.addNewColumnsForAggregation(GroupByNode.java:558) at org.apache.derby.impl.sql.compile.GroupByNode.addAggregates(GroupByNode.java:245) at org.apache.derby.impl.sql.compile.GroupByNode.init(GroupByNode.java:184) at org.apache.derby.iapi.sql.compile.NodeFactory.getNode(NodeFactory.java:273) at org.apache.derby.impl.sql.compile.SelectNode.genProjectRestrict(SelectNode.java:1481) at org.apache.derby.impl.sql.compile.SelectNode.modifyAccessPaths(SelectNode.java:2121) at org.apache.derby.impl.sql.compile.DMLStatementNode.optimizeStatement(DMLStatementNode.java:317) at org.apache.derby.impl.sql.compile.CursorNode.optimizeStatement(CursorNode.java:558) at org.apache.derby.impl.sql.GenericStatement.prepMinion(GenericStatement.java:381) at org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java:90) at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(GenericLanguageConnectionContext.java:843) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:606) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555)
          Hide
          Matt Doran added a comment -

          I should also add that removing the having clause allows the query to run successfully.

          Show
          Matt Doran added a comment - I should also add that removing the having clause allows the query to run successfully.
          Hide
          Matt Doran added a comment -

          Crash log from 10.4.2.0

          Show
          Matt Doran added a comment - Crash log from 10.4.2.0
          Hide
          Matt Doran added a comment -

          Crash log from 10.5.3.0

          Show
          Matt Doran added a comment - Crash log from 10.5.3.0
          Hide
          Matt Doran added a comment -

          Crash log from derby 10.6.1.0

          Show
          Matt Doran added a comment - Crash log from derby 10.6.1.0

            People

            • Assignee:
              Dag H. Wanvik
              Reporter:
              Matt Doran
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development