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. npe.sql
        0.3 kB
        Knut Anders Hatlen
      3. addASimpleTest.diff
        2 kB
        Bryan Pendleton

        Issue Links

          Activity

          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.
          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 (...) ------------------------------------------------------------------------
          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; }
          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.
          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.
          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.
          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.
          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.
          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.
          Hide
          Lily Wei added a comment -

          Reopen to 10.5 back port

          Show
          Lily Wei added a comment - Reopen to 10.5 back port
          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
          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.

            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