Derby
  1. Derby
  2. DERBY-4419

NullPointerException with INSERT INTO ... from UNION and identity columns

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.3.1.4, 10.4.1.3, 10.5.1.1, 10.5.3.0, 10.6.1.0
    • Fix Version/s: 10.5.3.1, 10.6.1.0
    • Component/s: SQL
    • Labels:
      None
    • Issue & fix info:
      Repro attached
    • Bug behavior facts:
      Regression

      Description

      The following sequence of statements works on 10.2.2.0 and earlier, and raises a NullPointerException with 10.3.1.4 and later:

      ij> create table t1(x int);
      0 rows inserted/updated/deleted
      ij> insert into t1 values 1,2;
      2 rows inserted/updated/deleted
      ij> create table t2(x int);
      0 rows inserted/updated/deleted
      ij> insert into t2 values 2,3;
      2 rows inserted/updated/deleted
      ij> create table t3(x int, y int generated always as identity);
      0 rows inserted/updated/deleted
      ij> insert into t3 select * from t1 union select * from t2;
      ERROR XJ001: Java exception: ': java.lang.NullPointerException'.

      1. use4425CodePatch.diff
        2 kB
        Bryan Pendleton
      2. addASimpleTest.diff
        2 kB
        Bryan Pendleton
      3. npe.sql
        0.3 kB
        Knut Anders Hatlen

        Issue Links

          Activity

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

          Stack trace:

          java.lang.NullPointerException
          at org.apache.derby.impl.sql.compile.ResultColumn.columnTypeAndLengthMatch(ResultColumn.java:1003)
          at org.apache.derby.impl.sql.compile.ResultColumnList.columnTypesAndLengthsMatch(ResultColumnList.java:1686)
          at org.apache.derby.impl.sql.compile.ResultSetNode.columnTypesAndLengthsMatch(ResultSetNode.java:932)
          at org.apache.derby.impl.sql.compile.UnionNode.addNewNodes(UnionNode.java:411)
          at org.apache.derby.impl.sql.compile.UnionNode.modifyAccessPaths(UnionNode.java:361)
          at org.apache.derby.impl.sql.compile.SingleChildResultSetNode.modifyAccessPaths(SingleChildResultSetNode.java:439)
          at org.apache.derby.impl.sql.compile.DMLStatementNode.optimizeStatement(DMLStatementNode.java:323)
          at org.apache.derby.impl.sql.compile.DMLStatementNode.optimizeStatement(DMLStatementNode.java:302)
          at org.apache.derby.impl.sql.compile.DMLModStatementNode.optimizeStatement(DMLModStatementNode.java:1736)
          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:828)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:606)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555)
          at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:329)

          Show
          Knut Anders Hatlen added a comment - Stack trace: java.lang.NullPointerException at org.apache.derby.impl.sql.compile.ResultColumn.columnTypeAndLengthMatch(ResultColumn.java:1003) at org.apache.derby.impl.sql.compile.ResultColumnList.columnTypesAndLengthsMatch(ResultColumnList.java:1686) at org.apache.derby.impl.sql.compile.ResultSetNode.columnTypesAndLengthsMatch(ResultSetNode.java:932) at org.apache.derby.impl.sql.compile.UnionNode.addNewNodes(UnionNode.java:411) at org.apache.derby.impl.sql.compile.UnionNode.modifyAccessPaths(UnionNode.java:361) at org.apache.derby.impl.sql.compile.SingleChildResultSetNode.modifyAccessPaths(SingleChildResultSetNode.java:439) at org.apache.derby.impl.sql.compile.DMLStatementNode.optimizeStatement(DMLStatementNode.java:323) at org.apache.derby.impl.sql.compile.DMLStatementNode.optimizeStatement(DMLStatementNode.java:302) at org.apache.derby.impl.sql.compile.DMLModStatementNode.optimizeStatement(DMLModStatementNode.java:1736) 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:828) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:606) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555) at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:329)
          Hide
          Knut Anders Hatlen added a comment -

          Attached script to reproduce the NPE.

          Show
          Knut Anders Hatlen added a comment - Attached script to reproduce the NPE.
          Knut Anders Hatlen made changes -
          Field Original Value New Value
          Attachment npe.sql [ 12422993 ]
          Knut Anders Hatlen made changes -
          Component/s SQL [ 11408 ]
          Hide
          Knut Anders Hatlen added a comment -

          It looks like the bug was introduced by this commit:

          ------------------------------------------------------------------------
          r487414 | bpendleton | 2006-12-15 02:01:14 +0100 (Fri, 15 Dec 2006) | 55 lines

          DERBY-1644: NPE when inserting values to tbl w/ identity col gen by default
          (...)
          ------------------------------------------------------------------------

          Show
          Knut Anders Hatlen added a comment - It looks like the bug was introduced by this commit: ------------------------------------------------------------------------ r487414 | bpendleton | 2006-12-15 02:01:14 +0100 (Fri, 15 Dec 2006) | 55 lines DERBY-1644 : NPE when inserting values to tbl w/ identity col gen by default (...) ------------------------------------------------------------------------
          Knut Anders Hatlen made changes -
          Link This issue relates to DERBY-1644 [ DERBY-1644 ]
          Hide
          Bryan Pendleton added a comment -

          Thanks for tracking this down, Knut. The test case is eerily similar to the DERBY-1644 case.

          I'm afraid I don't have a good guess about what's going wrong, or why the DERBY-1644 patch caused this problem.

          Are you intending to investigate this? I'll try to take a look, although not immediately.

          Show
          Bryan Pendleton added a comment - Thanks for tracking this down, Knut. The test case is eerily similar to the DERBY-1644 case. I'm afraid I don't have a good guess about what's going wrong, or why the DERBY-1644 patch caused this problem. Are you intending to investigate this? I'll try to take a look, although not immediately.
          Hide
          Knut Anders Hatlen added a comment -

          Hi Bryan,
          I don't have any immediate plans to look into it, but I may do so out of pure curiosity.
          If I do so, I'll post whatever I find here.

          Show
          Knut Anders Hatlen added a comment - Hi Bryan, I don't have any immediate plans to look into it, but I may do so out of pure curiosity. If I do so, I'll post whatever I find here.
          Hide
          Bryan Pendleton added a comment -

          The following diff makes the repro script pass. I haven't done any other testing yet.
          On the face of it, it seems reasonable to treat generated columns and autoincrement
          generated columns the same way in this code, so I'll try running more tests to see
          what I find.

          Index: java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
          ===================================================================
          — java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java (revision 826583)
          +++ java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java (working copy)
          @@ -1678,7 +1678,7 @@
          ResultColumn resultColumn = (ResultColumn) elementAt(index);

          /* Skip over generated columns */

          • if (resultColumn.isGenerated())
            + if (resultColumn.isGenerated() || resultColumn.isAutoincrementGenerated()) { continue; }
          Show
          Bryan Pendleton added a comment - The following diff makes the repro script pass. I haven't done any other testing yet. On the face of it, it seems reasonable to treat generated columns and autoincrement generated columns the same way in this code, so I'll try running more tests to see what I find. Index: java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java =================================================================== — java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java (revision 826583) +++ java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java (working copy) @@ -1678,7 +1678,7 @@ ResultColumn resultColumn = (ResultColumn) elementAt(index); /* Skip over generated columns */ if (resultColumn.isGenerated()) + if (resultColumn.isGenerated() || resultColumn.isAutoincrementGenerated()) { continue; }
          Bryan Pendleton made changes -
          Assignee Bryan Pendleton [ bryanpendleton ]
          Hide
          Bryan Pendleton added a comment -

          Regression tests of the above patch were clean.

          Attached patch proposal adds a simple test, using the
          script from the bug description.

          Show
          Bryan Pendleton added a comment - Regression tests of the above patch were clean. Attached patch proposal adds a simple test, using the script from the bug description.
          Bryan Pendleton made changes -
          Attachment addASimpleTest.diff [ 12423161 ]
          Hide
          Knut Anders Hatlen added a comment -

          The fix looks reasonable to me. +1 to commit.

          I noticed though that a NullPointerException is still thrown if the definition of T3.Y is changed to "GENERATED ALWAYS AS (X*2)". Probably a similar fix is needed for that case. Since this kind of generated columns wasn't introduced until 10.5, this is not a regression, so I'll file it as a separate bug.

          Show
          Knut Anders Hatlen added a comment - The fix looks reasonable to me. +1 to commit. I noticed though that a NullPointerException is still thrown if the definition of T3.Y is changed to "GENERATED ALWAYS AS (X*2)". Probably a similar fix is needed for that case. Since this kind of generated columns wasn't introduced until 10.5, this is not a regression, so I'll file it as a separate bug.
          Knut Anders Hatlen made changes -
          Link This issue is related to DERBY-4425 [ DERBY-4425 ]
          Hide
          Knut Anders Hatlen added a comment -

          Logged the GENERATED ALWAYS AS (X*2) case as DERBY-4425.

          Show
          Knut Anders Hatlen added a comment - Logged the GENERATED ALWAYS AS (X*2) case as DERBY-4425 .
          Hide
          Dag H. Wanvik added a comment -

          This one threw me off at first:

          /* Skip over generated columns */

          • if (resultColumn.isGenerated())
            + if (resultColumn.isGenerated() || resultColumn.isAutoincrementGenerated())

          In view of Knut's find, DERBY-4425, it seems that "generated column" in the code quoted means something else. Perhaps the term is overloaded in the code, which is unfortunate, and may lead to other issues later.
          Cf this code fragment later in the same class:

          defaultInfo.isGeneratedColumn()

          which is the test for real "generated column" in the SQL sense. From 4.14.8:

          "A generated column is one whose values are determined by evaluation of a generation
          expression.."

          So, the ResultColumn method "isGenerated" means something else, i.e. column synthesized by Derby for various purposes (cf calls to markGenerated in UpdateNode/DeleteNode vs. GroupByNode/WindowResultSetNode).

          Show
          Dag H. Wanvik added a comment - This one threw me off at first: /* Skip over generated columns */ if (resultColumn.isGenerated()) + if (resultColumn.isGenerated() || resultColumn.isAutoincrementGenerated()) In view of Knut's find, DERBY-4425 , it seems that "generated column" in the code quoted means something else. Perhaps the term is overloaded in the code, which is unfortunate, and may lead to other issues later. Cf this code fragment later in the same class: defaultInfo.isGeneratedColumn() which is the test for real "generated column" in the SQL sense. From 4.14.8: "A generated column is one whose values are determined by evaluation of a generation expression.." So, the ResultColumn method "isGenerated" means something else, i.e. column synthesized by Derby for various purposes (cf calls to markGenerated in UpdateNode/DeleteNode vs. GroupByNode/WindowResultSetNode).
          Hide
          Dag H. Wanvik added a comment -

          Just to be clear, +1, though, if we want to clean up terminology that would be another issue..

          Show
          Dag H. Wanvik added a comment - Just to be clear, +1, though, if we want to clean up terminology that would be another issue..
          Hide
          Bryan Pendleton added a comment -

          I agree, Dag, the terminology is worrisome here.

          There is also ResultColumn.isGeneratedForUnmatchedColumnInInsert()

          I think there is more to learn here; I shall study it some more.

          Show
          Bryan Pendleton added a comment - I agree, Dag, the terminology is worrisome here. There is also ResultColumn.isGeneratedForUnmatchedColumnInInsert() I think there is more to learn here; I shall study it some more.
          Hide
          Knut Anders Hatlen added a comment -

          Changed "generated columns" to "identity columns" in the bug summary so that we at least get the terminology right here.

          Show
          Knut Anders Hatlen added a comment - Changed "generated columns" to "identity columns" in the bug summary so that we at least get the terminology right here.
          Knut Anders Hatlen made changes -
          Summary NullPointerException with INSERT INTO ... from UNION and generated columns NullPointerException with INSERT INTO ... from UNION and identity columns
          Hide
          Bryan Pendleton added a comment -

          It looks like, indeed, the same code patch from DERBY-4425 will work here,
          of course with a different regression test.

          The full regression suite was clean.

          I'm intending to commit the fixes for DERBY-4419 and DERBY-4425, since
          they seem small, self-contained, and safe.

          I don't have a great resolution for the terminology confusion of the word "generated",
          which has come to have a variety of meanings inside the Derby SQL engine. Hopefully
          over time we can tease these details apart.

          In the meantime, I don't think that the proposed patch makes matters much worse.

          Show
          Bryan Pendleton added a comment - It looks like, indeed, the same code patch from DERBY-4425 will work here, of course with a different regression test. The full regression suite was clean. I'm intending to commit the fixes for DERBY-4419 and DERBY-4425 , since they seem small, self-contained, and safe. I don't have a great resolution for the terminology confusion of the word "generated", which has come to have a variety of meanings inside the Derby SQL engine. Hopefully over time we can tease these details apart. In the meantime, I don't think that the proposed patch makes matters much worse.
          Bryan Pendleton made changes -
          Attachment use4425CodePatch.diff [ 12423643 ]
          Hide
          Knut Anders Hatlen added a comment -

          +1. Looks like a simple and safe fix.

          Show
          Knut Anders Hatlen added a comment - +1. Looks like a simple and safe fix.
          Hide
          Bryan Pendleton added a comment -

          Committed to the trunk as revision 831304.

          I'm not planning to merge this back to prior branches, although I think it
          would be a straightforward merge if it interests anyone.

          Show
          Bryan Pendleton added a comment - Committed to the trunk as revision 831304. I'm not planning to merge this back to prior branches, although I think it would be a straightforward merge if it interests anyone.
          Bryan Pendleton made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 10.6.0.0 [ 12313727 ]
          Resolution Fixed [ 1 ]
          Dag H. Wanvik made changes -
          Link This issue relates to DERBY-4442 [ DERBY-4442 ]
          Hide
          Knut Anders Hatlen added a comment -

          DERBY-4442 fixed a family of INSERT bugs, including this one, making the original fix for this issue unnecessary. The fix was therefore backed out in revision 888311, but the tests are still part of the regression test suite.

          Show
          Knut Anders Hatlen added a comment - DERBY-4442 fixed a family of INSERT bugs, including this one, making the original fix for this issue unnecessary. The fix was therefore backed out in revision 888311, but the tests are still part of the regression test suite.
          Hide
          Knut Anders Hatlen added a comment -

          Verified on trunk. Closing.

          Show
          Knut Anders Hatlen added a comment - Verified on trunk. Closing.
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Lily Wei made changes -
          Link This issue is required by DERBY-4728 [ DERBY-4728 ]
          Hide
          Lily Wei added a comment -

          Reopen to 10.5 back port

          Show
          Lily Wei added a comment - Reopen to 10.5 back port
          Lily Wei made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Hide
          Mike Matrigali added a comment -

          working on backporting this issue to 10.5. DERBY-4419, DERBY-4413, DERBY-4425, and DERBY-4442 all seem related. I am going to apply and checkin the backported changes to these issues in order. I think the changes necessary are:
          DERBY-4413 #829410
          DERBY-4419 #831304
          DERBY-4425 #831319
          DERBY-4442 #885421
          DERBY-4413 #885659
          DERBY-4442 #888311

          Show
          Mike Matrigali added a comment - working on backporting this issue to 10.5. DERBY-4419 , DERBY-4413 , DERBY-4425 , and DERBY-4442 all seem related. I am going to apply and checkin the backported changes to these issues in order. I think the changes necessary are: DERBY-4413 #829410 DERBY-4419 #831304 DERBY-4425 #831319 DERBY-4442 #885421 DERBY-4413 #885659 DERBY-4442 #888311
          Mike Matrigali made changes -
          Assignee Bryan Pendleton [ bryanpendleton ] Mike Matrigali [ mikem ]
          Hide
          Mike Matrigali added a comment -

          backported change #831304 from trunk to 10.5 branch:

          m105_jdk16:46>svn commit
          Sending .
          ZSending java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
          Sending java/testing/org/apache/derbyTesting/functionTests/master/autoincrement.out
          Sending java/testing/org/apache/derbyTesting/functionTests/tests/lang/autoincrement.sql
          Transmitting file data ...
          Committed revision 960346.

          Show
          Mike Matrigali added a comment - backported change #831304 from trunk to 10.5 branch: m105_jdk16:46>svn commit Sending . ZSending java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java Sending java/testing/org/apache/derbyTesting/functionTests/master/autoincrement.out Sending java/testing/org/apache/derbyTesting/functionTests/tests/lang/autoincrement.sql Transmitting file data ... Committed revision 960346.
          Hide
          Mike Matrigali added a comment -

          backported all changes for this fix from trunk to 10.5, resolving as fixed in 10.5 and resetting original owner.

          Show
          Mike Matrigali added a comment - backported all changes for this fix from trunk to 10.5, resolving as fixed in 10.5 and resetting original owner.
          Mike Matrigali made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Assignee Mike Matrigali [ mikem ] Bryan Pendleton [ bryanpendleton ]
          Fix Version/s 10.5.3.1 [ 12314182 ]
          Resolution Fixed [ 1 ]
          Knut Anders Hatlen made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Workflow jira [ 12480244 ] Default workflow, editable Closed status [ 12798881 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          7d 7h 30m 1 Bryan Pendleton 30/Oct/09 14:12
          Closed Closed Reopened Reopened
          149d 12h 12m 1 Lily Wei 01/Jul/10 23:02
          Reopened Reopened Resolved Resolved
          4d 20h 51m 1 Mike Matrigali 06/Jul/10 19:53
          Resolved Resolved Closed Closed
          285d 18h 2m 2 Knut Anders Hatlen 13/Jan/11 17:18

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development