Derby
  1. Derby
  2. DERBY-4451

ArrayIndexOutOfBoundsException or ASSERT FAILED when inserting generated columns out of order

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.5.3.0, 10.6.1.0
    • Fix Version/s: 10.5.3.1, 10.6.1.0
    • Component/s: SQL
    • Labels:
      None

      Description

      I see this error when I specify the columns in a different order than in the table definition. It only fails if a multi-row table constructor is used.

      ij> create table t(a int, b generated always as (-a));
      0 rows inserted/updated/deleted
      ij> insert into t(b,a) values (default,1);
      1 row inserted/updated/deleted
      ij> insert into t(b,a) values (default,1), (default, 2);
      ERROR XJ001: Java exception: '1 >= 1: java.lang.ArrayIndexOutOfBoundsException'.

      And in a sane build:

      ij> insert into t(b,a) values (default,1),(default,2);
      ERROR XJ001: Java exception: 'ASSERT FAILED More columns in result column list than in base table: org.apache.derby.shared.common.sanity.AssertFailure'.

      This bug may be similar to DERBY-4448, but the stack trace is different, and DERBY-4448 does not raise an ASSERT FAILED in sane builds.

      1. derby-4451d.stat
        0.3 kB
        Dag H. Wanvik
      2. derby-4451d.diff
        12 kB
        Dag H. Wanvik
      3. derby-4451c.stat
        0.5 kB
        Dag H. Wanvik
      4. derby-4451c.diff
        17 kB
        Dag H. Wanvik
      5. derby-4451b.diff
        11 kB
        Dag H. Wanvik
      6. derby-4451b.stat
        0.4 kB
        Dag H. Wanvik
      7. derby-4451.stat
        0.4 kB
        Dag H. Wanvik
      8. derby-4451.diff
        11 kB
        Dag H. Wanvik

        Activity

        Hide
        Knut Anders Hatlen added a comment -

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

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

        Verified fix on trunk. Closing.

        Show
        Knut Anders Hatlen added a comment - Verified fix on trunk. Closing.
        Hide
        Dag H. Wanvik added a comment -

        Backported to 10.5 as svn 884738 (thanks to Knut for backporting DERBY-4420).

        Show
        Dag H. Wanvik added a comment - Backported to 10.5 as svn 884738 (thanks to Knut for backporting DERBY-4420 ).
        Hide
        Knut Anders Hatlen added a comment -

        I'll look into backporting DERBY-4420 to 10.5.

        Show
        Knut Anders Hatlen added a comment - I'll look into backporting DERBY-4420 to 10.5.
        Hide
        Dag H. Wanvik added a comment - - edited

        Investigated backporting this to 10.5, but GeneratedColumnsTest broke due to a dependency of DERBY-4420 which has not yet been backported.

        Show
        Dag H. Wanvik added a comment - - edited Investigated backporting this to 10.5, but GeneratedColumnsTest broke due to a dependency of DERBY-4420 which has not yet been backported.
        Hide
        Dag H. Wanvik added a comment -

        Commited derby-4451d on trunk as svn 884163, resolving. I leave it open for backporting to 10.5

        Show
        Dag H. Wanvik added a comment - Commited derby-4451d on trunk as svn 884163, resolving. I leave it open for backporting to 10.5
        Hide
        Knut Anders Hatlen added a comment -

        The patch looks good to me. I tried something similar in DERBY-4448 (see comment dated 19/Nov/09 08:12 AM) but for some reason that I don't remember now, I also made some changes to when InsertNode.enhanceAndCheckForAutoincrement() was called. This caused a failure in GeneratedColumnsTest. I'm trying to remember why I thought that change was needed in addition to removing the call to forbidGenerationOverrides(), but nothing comes to mind, so I'm guessing I just wasn't thinking clearly at that moment. +1 to commit if the regression tests ran cleanly.

        Show
        Knut Anders Hatlen added a comment - The patch looks good to me. I tried something similar in DERBY-4448 (see comment dated 19/Nov/09 08:12 AM) but for some reason that I don't remember now, I also made some changes to when InsertNode.enhanceAndCheckForAutoincrement() was called. This caused a failure in GeneratedColumnsTest. I'm trying to remember why I thought that change was needed in addition to removing the call to forbidGenerationOverrides(), but nothing comes to mind, so I'm guessing I just wasn't thinking clearly at that moment. +1 to commit if the regression tests ran cleanly.
        Hide
        Dag H. Wanvik added a comment -

        Discussed this offline with Knut, it turns out that the removal of generated columns for the case where we have an explicit target column list is not really required. This frees us from having to duplicate the checking if nested DEFAULTs at this stage also, so we can devise a solution which works for all column types; postponed this to DERBY-4426.
        The resulting patch is way simpler now:

        • drop the call to forbidGenerationOverrides from InsertNode.
        • move forbidGenerationOverrides to UpdateNode since it's now only used by that class.
        • simplified forbidGenerationOverrides to only handle the update case
        • removed the test cases for DERBY-4426 from GeneratedColumnsTest seen in earlier patch versions.
        • removed forbidGenerationOverrides from the result sets seen in earlier patch versions.

        Rerunning regressions.

        Show
        Dag H. Wanvik added a comment - Discussed this offline with Knut, it turns out that the removal of generated columns for the case where we have an explicit target column list is not really required. This frees us from having to duplicate the checking if nested DEFAULTs at this stage also, so we can devise a solution which works for all column types; postponed this to DERBY-4426 . The resulting patch is way simpler now: drop the call to forbidGenerationOverrides from InsertNode. move forbidGenerationOverrides to UpdateNode since it's now only used by that class. simplified forbidGenerationOverrides to only handle the update case removed the test cases for DERBY-4426 from GeneratedColumnsTest seen in earlier patch versions. removed forbidGenerationOverrides from the result sets seen in earlier patch versions. Rerunning regressions.
        Hide
        Knut Anders Hatlen added a comment -

        If I understand correctly, the patch will disallow DEFAULT in EXCEPT/INTERSECT/UNION, but only for the queries that end up calling ResultSetNode.forbidGenerationOverrides(). This means that it will be disallowed if the DEFAULT is specified for a generated column (not for identity columns or regular columns), and an explicit column list is specified. So given a table

        CREATE TABLE T(A INT, B GENERATED ALWAYS AS (-A));

        the following is disallowed:

        INSERT INTO T(B) VALUES (DEFAULT) UNION VALUES (DEFAULT);
        INSERT INTO T(A, B) VALUES (DEFAULT, DEFAULT) UNION VALUES (DEFAULT, DEFAULT);

        whereas this is still allowed:

        INSERT INTO T(A) VALUES (DEFAULT) UNION VALUES (DEFAULT);
        INSERT INTO T VALUES (DEFAULT, DEFAULT) UNION VALUES (DEFAULT, DEFAULT);

        Is that about right? If so, using this mechanism to disallow DEFAULT seems to miss most cases, so perhaps it's the wrong place to add this check? Since the checking makes the patch somewhat more complex, and it is not actually needed for this issue (is it?), perhaps it's better to leave it out and try do devise a complete fix in DERBY-4426 instead?

        Show
        Knut Anders Hatlen added a comment - If I understand correctly, the patch will disallow DEFAULT in EXCEPT/INTERSECT/UNION, but only for the queries that end up calling ResultSetNode.forbidGenerationOverrides(). This means that it will be disallowed if the DEFAULT is specified for a generated column (not for identity columns or regular columns), and an explicit column list is specified. So given a table CREATE TABLE T(A INT, B GENERATED ALWAYS AS (-A)); the following is disallowed: INSERT INTO T(B) VALUES (DEFAULT) UNION VALUES (DEFAULT); INSERT INTO T(A, B) VALUES (DEFAULT, DEFAULT) UNION VALUES (DEFAULT, DEFAULT); whereas this is still allowed: INSERT INTO T(A) VALUES (DEFAULT) UNION VALUES (DEFAULT); INSERT INTO T VALUES (DEFAULT, DEFAULT) UNION VALUES (DEFAULT, DEFAULT); Is that about right? If so, using this mechanism to disallow DEFAULT seems to miss most cases, so perhaps it's the wrong place to add this check? Since the checking makes the patch somewhat more complex, and it is not actually needed for this issue (is it?), perhaps it's better to leave it out and try do devise a complete fix in DERBY-4426 instead?
        Hide
        Dag H. Wanvik added a comment -

        Uploading derby-4451c, which fixes the case seen by Knut. It also now handles INTERSECT/EXCEPT and any nested use of DEFAULT, i.e. use of DEFAULT is only allowed in a top level table constructor. Note that this does not solve DERBY-4426, which does not have an explicit target column list, and so is not checked by forbidGenerationOverrides#forbidGenerationOverrides. We do need to add this logic here, though, for otherwise we would lose the information that the user provided a default in a nested table constructor (it would be pruned away). The alternative would be to just throw LANG_CANT_OVERRIDE_GENERATION_CLAUSE, but that could be confusing for the following query:

        insert into t(a,b) values (3,default) union all values (3,default)

        Added more test cases, rerunning regressions.

        Show
        Dag H. Wanvik added a comment - Uploading derby-4451c, which fixes the case seen by Knut. It also now handles INTERSECT/EXCEPT and any nested use of DEFAULT, i.e. use of DEFAULT is only allowed in a top level table constructor. Note that this does not solve DERBY-4426 , which does not have an explicit target column list, and so is not checked by forbidGenerationOverrides#forbidGenerationOverrides. We do need to add this logic here, though, for otherwise we would lose the information that the user provided a default in a nested table constructor (it would be pruned away). The alternative would be to just throw LANG_CANT_OVERRIDE_GENERATION_CLAUSE, but that could be confusing for the following query: insert into t(a,b) values (3,default) union all values (3,default) Added more test cases, rerunning regressions.
        Hide
        Dag H. Wanvik added a comment - - edited

        Thanks for looking at the patch, guys. Interestingly,

        insert into t(x,y) select * from t union select * from t;

        gives the ArrayIndexOutOfBoundsException, but this version:

        ij(CONNECTION1)> insert into t(y,x) select * from t union select * from t;
        ERROR 42XA3: You may not override the value of generated column 'Y'.

        works as expected. I also see that the patch doesn't handle INTERSECT/EXCEPT properly, but that's easily fixed by moving the mothod from UnionNode up to SetOperatorNode. I'll spin a new verision when I have figured out the ArrayIndexOutOfBoundsException case. Another thing is that the new version would silently remove the defaults in this case (DERBY-4426 variant):

        create table mytab (i int generated always as (j*2), j int);
        insert into mytab(i,j) values (default,1) union values (default,2);

        wheres they should be flagged according to the discussion in DERBY-4426..

        Show
        Dag H. Wanvik added a comment - - edited Thanks for looking at the patch, guys. Interestingly, insert into t(x,y) select * from t union select * from t; gives the ArrayIndexOutOfBoundsException, but this version: ij(CONNECTION1)> insert into t(y,x) select * from t union select * from t; ERROR 42XA3: You may not override the value of generated column 'Y'. works as expected. I also see that the patch doesn't handle INTERSECT/EXCEPT properly, but that's easily fixed by moving the mothod from UnionNode up to SetOperatorNode. I'll spin a new verision when I have figured out the ArrayIndexOutOfBoundsException case. Another thing is that the new version would silently remove the defaults in this case ( DERBY-4426 variant): create table mytab (i int generated always as (j*2), j int); insert into mytab(i,j) values (default,1) union values (default,2); wheres they should be flagged according to the discussion in DERBY-4426 ..
        Hide
        Knut Anders Hatlen added a comment -

        The approach in the patch looks good. Some small comments:

        • to get consistent naming of the forbid* methods, you may want to rename the new method to forbidGenerationOverrides (note: lower-case b in forbid)
        • typo in DMLModStatementNode: multi-row VALUE clause -> multi-row VALUES clause
        • the code touched in DMLModStatementNode uses spaces for indentation, whereas the added code uses tabs
        • javadoc for ResultSetNode.forBidGenerationOverrides() says "For a UnionNode representing a table constructor, this happens recursively till we have checked all rows", but in the implementation, UnionNodes that are not representing a table constructor are handled the same way.
        Show
        Knut Anders Hatlen added a comment - The approach in the patch looks good. Some small comments: to get consistent naming of the forbid* methods, you may want to rename the new method to forbidGenerationOverrides (note: lower-case b in forbid) typo in DMLModStatementNode: multi-row VALUE clause -> multi-row VALUES clause the code touched in DMLModStatementNode uses spaces for indentation, whereas the added code uses tabs javadoc for ResultSetNode.forBidGenerationOverrides() says "For a UnionNode representing a table constructor, this happens recursively till we have checked all rows", but in the implementation, UnionNodes that are not representing a table constructor are handled the same way.
        Hide
        Kristian Waagan added a comment -

        FYI, I tried the repro without the union. I can confirm that the one with the union fails for me too, as Knut reported.

        Show
        Kristian Waagan added a comment - FYI, I tried the repro without the union. I can confirm that the one with the union fails for me too, as Knut reported.
        Hide
        Knut Anders Hatlen added a comment -

        Thanks for the patch. I still see this ArrayIndexOutOfBoundsException, though, which appears to be another instance of DERBY-4448:

        ij> create table t(x int, y generated always as (-x));
        0 rows inserted/updated/deleted
        ij> insert into t(x,y) select * from t union select * from t;
        ERROR XJ001: Java exception: '1 >= 1: java.lang.ArrayIndexOutOfBoundsException'.

        Show
        Knut Anders Hatlen added a comment - Thanks for the patch. I still see this ArrayIndexOutOfBoundsException, though, which appears to be another instance of DERBY-4448 : ij> create table t(x int, y generated always as (-x)); 0 rows inserted/updated/deleted ij> insert into t(x,y) select * from t union select * from t; ERROR XJ001: Java exception: '1 >= 1: java.lang.ArrayIndexOutOfBoundsException'.
        Hide
        Kristian Waagan added a comment -

        I ran the regression tests (no failures) and tried Knut's repro. It failed without the patch, and passed with the patch.
        This area of the code is new to me, so I would need more time to fully understand the code changes.

        Show
        Kristian Waagan added a comment - I ran the regression tests (no failures) and tried Knut's repro. It failed without the patch, and passed with the patch. This area of the code is new to me, so I would need more time to fully understand the code changes.
        Hide
        Dag H. Wanvik added a comment -

        Uploading a slightly improved version, derby-4451b.

        Show
        Dag H. Wanvik added a comment - Uploading a slightly improved version, derby-4451b.
        Hide
        Dag H. Wanvik added a comment -

        Regressions passed, please review.

        Show
        Dag H. Wanvik added a comment - Regressions passed, please review.
        Hide
        Dag H. Wanvik added a comment - - edited

        Uploading a fix for this, which also solves DERBY-4448 which has the same underlying cause;
        the checking and pruning of explictly given generated columns (only default is allowed), fails to visit all rows in a multi-row VALUES clause. The fix moves the source result set part of INSERT logic in DMLModStatementNode#forbidGenerationOverrides down to the relevant result sets, and adds tests for DERBY-4448 as well as this issue to GeneratedColumnsTest. Running regressions.

        Show
        Dag H. Wanvik added a comment - - edited Uploading a fix for this, which also solves DERBY-4448 which has the same underlying cause; the checking and pruning of explictly given generated columns (only default is allowed), fails to visit all rows in a multi-row VALUES clause. The fix moves the source result set part of INSERT logic in DMLModStatementNode#forbidGenerationOverrides down to the relevant result sets, and adds tests for DERBY-4448 as well as this issue to GeneratedColumnsTest. Running regressions.

          People

          • Assignee:
            Dag H. Wanvik
            Reporter:
            Knut Anders Hatlen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development