Derby
  1. Derby
  2. DERBY-4244

ALTER TABLE Sanity ASSERT in add column with autocommit off

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.2.1.6, 10.2.2.0, 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.6.1.0
    • Fix Version/s: 10.5.3.1, 10.6.1.0
    • Component/s: SQL
    • Labels:
      None
    • Urgency:
      Normal
    • Issue & fix info:
      Repro attached

      Description

      While working with Eranda on DERBY-4187, I stumbled across an apparent ALTER TABLE bug.

      Here's a script which reproduces the problem for me:

      autocommit off;
      create table t0(c1 int not null constraint p1 primary key);
      alter table t0 add column c1 int;
      alter table t0 add column c2 int not null default 0 primary key;
      alter table t0 add column c2 int not null default 0;

      The "autocommit off" is crucial; otherwise the problem does not reproduce.

      Here's the detailed assertion failure:

      2009-05-23 15:01:17.436 GMT Thread[main,5,main] (XID = 146), (SESSIONID = 1), (DATABASE = brydb), (DRDAID = null), Failed Statement is: alter table t0 add column c2 int not null default 0
      org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED column_id = 1format_ids.length = 2format_ids = [I@1321f5
      at org.apache.derby.shared.common.sanity.SanityManager.THROWASSERT(SanityManager.java:162)
      at org.apache.derby.shared.common.sanity.SanityManager.THROWASSERT(SanityManager.java:147)
      at org.apache.derby.impl.store.access.heap.Heap.addColumn(Heap.java:418)
      at org.apache.derby.impl.store.access.RAMTransaction.addColumnToConglomerate(RAMTransaction.java:618)
      at org.apache.derby.impl.sql.execute.AlterTableConstantAction.addNewColumnToTable(AlterTableConstantAction.java:1325)
      at org.apache.derby.impl.sql.execute.AlterTableConstantAction.executeConstantAction(AlterTableConstantAction.java:449)

      Here's the relevant section of Heap.java:

      if (column_id != format_ids.length)

      { if (SanityManager.DEBUG) SanityManager.THROWASSERT( "column_id = " + column_id + "format_ids.length = " + format_ids.length + "format_ids = " + format_ids); throw(StandardException.newException( SQLState.HEAP_TEMPLATE_MISMATCH, new Long(column_id), new Long(this.format_ids.length))); }
      1. checkAtCompileTime.diff
        2 kB
        Bryan Pendleton
      2. ASF.LICENSE.NOT.GRANTED--DERBY-4244.diff
        5 kB
        Eranda Sooriyabandara

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          Does the THROWASSERT add any value in this code? It seems like all useful information is included in the message of the StandardException that will be thrown otherwise (I don't regard the identity hash code of the int array useful, and that's the only missing piece, as far as I can see). Not that removing the THROWASSERT will fix the bug...

          Show
          Knut Anders Hatlen added a comment - Does the THROWASSERT add any value in this code? It seems like all useful information is included in the message of the StandardException that will be thrown otherwise (I don't regard the identity hash code of the int array useful, and that's the only missing piece, as far as I can see). Not that removing the THROWASSERT will fix the bug...
          Hide
          Eranda Sooriyabandara added a comment -

          Hi,
          I remove,
          SanityManager.THROWASSERT(
          "column_id = " + column_id +
          "format_ids.length = " + format_ids.length +
          "format_ids = " + format_ids);

          and I get the new error as,
          Caused by: ERROR XSCH5: In a base table there was a mismatch between
          the requested column number 1 and the maximum numbe
          r of columns 2.
          at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:303)
          at org.apache.derby.impl.store.access.heap.Heap.addColumn(Heap.java:423)
          at org.apache.derby.impl.store.access.RAMTransaction.addColumnToConglomerate(RAMTransaction.java:618)
          at org.apache.derby.impl.sql.execute.AlterTableConstantAction.addNewColumnToTable(AlterTableConstantAction.java:
          1325)
          at org.apache.derby.impl.sql.execute.AlterTableConstantAction.executeConstantAction(AlterTableConstantAction.java:449)
          at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64)

          Thanks
          Eranda

          Show
          Eranda Sooriyabandara added a comment - Hi, I remove, SanityManager.THROWASSERT( "column_id = " + column_id + "format_ids.length = " + format_ids.length + "format_ids = " + format_ids); and I get the new error as, Caused by: ERROR XSCH5: In a base table there was a mismatch between the requested column number 1 and the maximum numbe r of columns 2. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:303) at org.apache.derby.impl.store.access.heap.Heap.addColumn(Heap.java:423) at org.apache.derby.impl.store.access.RAMTransaction.addColumnToConglomerate(RAMTransaction.java:618) at org.apache.derby.impl.sql.execute.AlterTableConstantAction.addNewColumnToTable(AlterTableConstantAction.java: 1325) at org.apache.derby.impl.sql.execute.AlterTableConstantAction.executeConstantAction(AlterTableConstantAction.java:449) at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64) Thanks Eranda
          Hide
          Bryan Pendleton added a comment -

          Hi Eranda,

          In the script which reproduces the problem, I believe it is the third ALTER TABLE
          statement which crashes, is that correct?

          What happens if you remove the first or the second ALTER TABLE statements in
          the script? Does it still fail the same way?

          Also, what happens if you start with the original script, but in the third ALTER TABLE
          statement you change "c2" to "c3"? Does it still fail the same way?

          Show
          Bryan Pendleton added a comment - Hi Eranda, In the script which reproduces the problem, I believe it is the third ALTER TABLE statement which crashes, is that correct? What happens if you remove the first or the second ALTER TABLE statements in the script? Does it still fail the same way? Also, what happens if you start with the original script, but in the third ALTER TABLE statement you change "c2" to "c3"? Does it still fail the same way?
          Hide
          Bryan Pendleton added a comment -

          Hi Eranda,

          If you run the reproduction script with derby.language.statementCacheSize=0,
          does the problem still occur?

          Show
          Bryan Pendleton added a comment - Hi Eranda, If you run the reproduction script with derby.language.statementCacheSize=0, does the problem still occur?
          Hide
          Dag H. Wanvik added a comment -

          Triaged for 10.5.2, setting "repro attached" and "normal" urgency.
          Saw this back to 10.2, setting affected flags accordingly.

          Show
          Dag H. Wanvik added a comment - Triaged for 10.5.2, setting "repro attached" and "normal" urgency. Saw this back to 10.2, setting affected flags accordingly.
          Hide
          Eranda Sooriyabandara added a comment -

          Hi Bryan,Thanks for helping me to figure this out.
          This error is causing by the previous command
          without this command the test runs successfully.
          When I change the command to,
          the test was successful.
          I think there must be remaining part of the previous statement in some
          where. But,
          according to the test setting derby.language.statementCacheSize=0 did
          "not" change
          the behavior of the bug.
          And it means that the remaining part of the statement is not in the cache.
          We can solve the bug if we can find where it get stored?
          Am I correct? Are there any misunderstanding of me?

          Show
          Eranda Sooriyabandara added a comment - Hi Bryan,Thanks for helping me to figure this out. This error is causing by the previous command without this command the test runs successfully. When I change the command to, the test was successful. I think there must be remaining part of the previous statement in some where. But, according to the test setting derby.language.statementCacheSize=0 did "not" change the behavior of the bug. And it means that the remaining part of the statement is not in the cache. We can solve the bug if we can find where it get stored? Am I correct? Are there any misunderstanding of me?
          Hide
          Bryan Pendleton added a comment -

          If I change the next-to-last SQL statement from

          alter table t0 add column c2 int not null default 0 primary key;

          to

          alter table t0 add column c2 int not null default 0;

          then the problem does not occur. This suggests to me that the problem involves
          the cleanup that is performed after this error is encountered

          ERROR X0Y58: Attempt to add a primary key constraint to table '"APP"."T0"' failed because the table already has a constraint of that type. A table can only have a single primary key constraint.

          I can see in derby.log that the stack trace at the time of this error looks like this:

          at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:286)
          at org.apache.derby.impl.sql.execute.AlterTableConstantAction.executeConstantAction(AlterTableConstantAction.java:541)
          at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64)
          at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:416)
          at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:297)
          at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1235)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625)
          at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555)
          at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:329)
          at org.apache.derby.impl.tools.ij.utilMain.doCatch(utilMain.java:521)
          at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(utilMain.java:363)
          at org.apache.derby.impl.tools.ij.utilMain.go(utilMain.java:261)
          at org.apache.derby.impl.tools.ij.Main.go(Main.java:222)
          at org.apache.derby.impl.tools.ij.Main.mainCore(Main.java:177)
          at org.apache.derby.impl.tools.ij.Main.main(Main.java:73)
          at org.apache.derby.tools.ij.main(ij.java:59)

          This suggests to me that the next step is to study the error handling that is performed in this
          part of the code; specifically whether AlterTableConstantAction.executeConstantAction isn't
          properly cleaning everything up when the error occurs.

          Show
          Bryan Pendleton added a comment - If I change the next-to-last SQL statement from alter table t0 add column c2 int not null default 0 primary key; to alter table t0 add column c2 int not null default 0; then the problem does not occur. This suggests to me that the problem involves the cleanup that is performed after this error is encountered ERROR X0Y58: Attempt to add a primary key constraint to table '"APP"."T0"' failed because the table already has a constraint of that type. A table can only have a single primary key constraint. I can see in derby.log that the stack trace at the time of this error looks like this: at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:286) at org.apache.derby.impl.sql.execute.AlterTableConstantAction.executeConstantAction(AlterTableConstantAction.java:541) at org.apache.derby.impl.sql.execute.MiscResultSet.open(MiscResultSet.java:64) at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(GenericPreparedStatement.java:416) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:297) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1235) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:625) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:555) at org.apache.derby.impl.tools.ij.ij.executeImmediate(ij.java:329) at org.apache.derby.impl.tools.ij.utilMain.doCatch(utilMain.java:521) at org.apache.derby.impl.tools.ij.utilMain.runScriptGuts(utilMain.java:363) at org.apache.derby.impl.tools.ij.utilMain.go(utilMain.java:261) at org.apache.derby.impl.tools.ij.Main.go(Main.java:222) at org.apache.derby.impl.tools.ij.Main.mainCore(Main.java:177) at org.apache.derby.impl.tools.ij.Main.main(Main.java:73) at org.apache.derby.tools.ij.main(ij.java:59) This suggests to me that the next step is to study the error handling that is performed in this part of the code; specifically whether AlterTableConstantAction.executeConstantAction isn't properly cleaning everything up when the error occurs.
          Hide
          Eranda Sooriyabandara added a comment -

          Hi Bryan,
          I did some tests, and get the results as follows

          ij> create table t0(c1 int not null constraint p1 primary key);

          0 rows inserted/updated/deleted

          ij> alter table t0 add column c2 int not null default 0 primary key;

          ERROR X0Y58: Attempt to add a primary key constraint to table '"APP"."T0"'
          failed because the table already has a constr

          aint of that type. A table can only have a single primary key constraint.

          ij> alter table t0 add column c3 int not null default 0;

          ERROR XSCH5: In a base table there was a mismatch between the requested
          column number 1 and the maximum number of column

          s 2.

          ij> alter table t0 add column c6 int not null default 0;

          ERROR XSCH5: In a base table there was a mismatch between the requested
          column number 1 and the maximum number of column

          s 2.

          ij> alter table t0 add column g6 int not null default 0;

          ERROR XSCH5: In a base table there was a mismatch between the requested
          column number 1 and the maximum number of column

          The test says that by "alter table t0 add column c2 int not null default 0
          primary key;" fails it self and also fails other statement of add columns.

          Thanks

          Eranda

          Show
          Eranda Sooriyabandara added a comment - Hi Bryan, I did some tests, and get the results as follows ij> create table t0(c1 int not null constraint p1 primary key); 0 rows inserted/updated/deleted ij> alter table t0 add column c2 int not null default 0 primary key; ERROR X0Y58: Attempt to add a primary key constraint to table '"APP"."T0"' failed because the table already has a constr aint of that type. A table can only have a single primary key constraint. ij> alter table t0 add column c3 int not null default 0; ERROR XSCH5: In a base table there was a mismatch between the requested column number 1 and the maximum number of column s 2. ij> alter table t0 add column c6 int not null default 0; ERROR XSCH5: In a base table there was a mismatch between the requested column number 1 and the maximum number of column s 2. ij> alter table t0 add column g6 int not null default 0; ERROR XSCH5: In a base table there was a mismatch between the requested column number 1 and the maximum number of column The test says that by "alter table t0 add column c2 int not null default 0 primary key;" fails it self and also fails other statement of add columns. Thanks Eranda
          Hide
          Bryan Pendleton added a comment -

          I spent some time stepping through the code and thinking about it.

          The X0Y58 message is generated at line 541 of AlterTableConstantAction.java:

          if (cdl.getPrimaryKey() != null)

          { throw StandardException.newException( SQLState.LANG_ADD_PRIMARY_KEY_FAILED1, td.getQualifiedName()); }

          However, this error is generated after the new column has already been added
          to the table, which occurs at line 450 of AlterTableConstantAction.java:

          if (columnInfo[ix].action == ColumnInfo.CREATE)

          { addNewColumnToTable(activation, lcc, dd, tc, ix); }

          addnewColumnToTable adds the new column to the table, then updates the
          data in the new column to the default value, using a nested sub-statement. This
          occurs at line 3199 of AlterTableConstantAction.java:

          String updateStmt = "UPDATE \"" + td.getSchemaName() + "\".\"" +
          td.getName() + "\" SET \"" +
          columnName + "\" = " + defaultText;

          AlterTableConstantAction.executeUpdate(lcc, updateStmt);

          private static void executeUpdate(LanguageConnectionContext lcc, String updateStmt) throws StandardException

          { PreparedStatement ps = lcc.prepareInternalStatement(updateStmt); // This is a substatement; for now, we do not set any timeout // for it. We might change this behaviour later, by linking // timeout to its parent statement's timeout settings. ResultSet rs = ps.executeSubStatement(lcc, true, 0L); rs.close(); }

          So the stack trace, when this is going on, looks something like:

          GenericPreparedStatements.executeStmt:443
          GenericPreparedStatement.executeSubStatement:272
          AlterTableConstantAction.executeUpdate:3209
          AlterTableConstantAction.updateNewColumnToDefault:3199
          AlterTableConstantAction.addNewColumnToTable:1374
          AlterTableConstantAction.executeConstantAction:450
          MiscResultSet.open:64
          GenericPreparedStatement.executeStmt:416
          GenericPreparedStatement.execute:297
          EmbedStatement.executeStatement:1235

          This leads me to two ideas:

          1) Although the X0Y58 error causes the outer statement to be rolled back to
          its savepoint, there must be something that occurs in the nested inner
          sub-statement which isn't undone by rolling back the outer statement to
          its savepoint. Specifically, something involving the descriptor information
          in the DataDictionary. So we could try to investigate that more.

          2) It seems odd that we only catch this error at execution time, after
          we have already added the new column to the table, and set all its
          default values, and only then do we notice that there's an error in the constraint.
          I think that the duplicate constraint error should be something that we are
          able to catch at compile time. So we could investigate the compile time
          processing of ALTER TABLE ADD COLUMN, and try to figure out why
          this error isn't being caught during compilation.

          Show
          Bryan Pendleton added a comment - I spent some time stepping through the code and thinking about it. The X0Y58 message is generated at line 541 of AlterTableConstantAction.java: if (cdl.getPrimaryKey() != null) { throw StandardException.newException( SQLState.LANG_ADD_PRIMARY_KEY_FAILED1, td.getQualifiedName()); } However, this error is generated after the new column has already been added to the table, which occurs at line 450 of AlterTableConstantAction.java: if (columnInfo [ix] .action == ColumnInfo.CREATE) { addNewColumnToTable(activation, lcc, dd, tc, ix); } addnewColumnToTable adds the new column to the table, then updates the data in the new column to the default value, using a nested sub-statement. This occurs at line 3199 of AlterTableConstantAction.java: String updateStmt = "UPDATE \"" + td.getSchemaName() + "\".\"" + td.getName() + "\" SET \"" + columnName + "\" = " + defaultText; AlterTableConstantAction.executeUpdate(lcc, updateStmt); private static void executeUpdate(LanguageConnectionContext lcc, String updateStmt) throws StandardException { PreparedStatement ps = lcc.prepareInternalStatement(updateStmt); // This is a substatement; for now, we do not set any timeout // for it. We might change this behaviour later, by linking // timeout to its parent statement's timeout settings. ResultSet rs = ps.executeSubStatement(lcc, true, 0L); rs.close(); } So the stack trace, when this is going on, looks something like: GenericPreparedStatements.executeStmt:443 GenericPreparedStatement.executeSubStatement:272 AlterTableConstantAction.executeUpdate:3209 AlterTableConstantAction.updateNewColumnToDefault:3199 AlterTableConstantAction.addNewColumnToTable:1374 AlterTableConstantAction.executeConstantAction:450 MiscResultSet.open:64 GenericPreparedStatement.executeStmt:416 GenericPreparedStatement.execute:297 EmbedStatement.executeStatement:1235 This leads me to two ideas: 1) Although the X0Y58 error causes the outer statement to be rolled back to its savepoint, there must be something that occurs in the nested inner sub-statement which isn't undone by rolling back the outer statement to its savepoint. Specifically, something involving the descriptor information in the DataDictionary. So we could try to investigate that more. 2) It seems odd that we only catch this error at execution time, after we have already added the new column to the table, and set all its default values, and only then do we notice that there's an error in the constraint. I think that the duplicate constraint error should be something that we are able to catch at compile time. So we could investigate the compile time processing of ALTER TABLE ADD COLUMN, and try to figure out why this error isn't being caught during compilation.
          Hide
          Bryan Pendleton added a comment -

          Hi Eranda,

          Attached code change 'checkAtCompileTime.diff' is a possible fix to pursue. This change
          adds some code to the compilation processing for ALTER TABLE to verify that we aren't
          adding a duplicate primary key to the table.

          I haven't done much testing, just verified that it fixes the 4-line script in the bug description.

          Can you have a look at this change and see what you think? If it seems reasonable, maybe
          you can do some more testing, and add some additional regression tests into the patch
          to turn it into a more complete patch proposal.

          Show
          Bryan Pendleton added a comment - Hi Eranda, Attached code change 'checkAtCompileTime.diff' is a possible fix to pursue. This change adds some code to the compilation processing for ALTER TABLE to verify that we aren't adding a duplicate primary key to the table. I haven't done much testing, just verified that it fixes the 4-line script in the bug description. Can you have a look at this change and see what you think? If it seems reasonable, maybe you can do some more testing, and add some additional regression tests into the patch to turn it into a more complete patch proposal.
          Hide
          Bryan Pendleton added a comment -

          Hi Eranda,

          Here's a few comments to hopefully explain how the attached patch works:

          During parsing, the parser processes the ALTER TABLE statement and breaks
          it down into pieces, then builds up a AlterTableNode instance to describe the statement.

          For an ALTER TABLE ADD ... statement, information about the column to be added, if any,
          and about the constraints that apply to that column, are stored in the table element list.
          This occurs at about lines 12622-12630 of sqlgrammar.jj, in the code:

          <ADD>
          (
          tableElement = addColumnDefinition(tableElementList)

          tableElement = tableConstraintDefinition()
          )
          {
          if (tableElement instanceof ColumnDefinitionNode)

          { //bug 5724 - auto increment columns not allowed in ALTER TABLE statement ColumnDefinitionNode cdn = (ColumnDefinitionNode) tableElement; if ( cdn.isAutoincrementColumn()) throw StandardException.newException(SQLState.LANG_ALTER_TABLE_AUTOINCREMENT_COLUMN_NOT_ALLOWED); }

          changeType[0] = DDLStatementNode.ADD_TYPE;
          tableElementList.addTableElement(tableElement);

          Later in the compilation of the ALTER TABLE statement, we call the makeConstantAction
          method in AlterTableNode.java, which in turn calls the prepConstantAction subroutine,
          at line 510 or so in AlterTableNode.java, which is the place where my proposed patch
          inserts some code.

          The first part of that new code looks like this:

          for (int conIndex = 0; conIndex < conActions.length; conIndex++)
          {
          ConstraintConstantAction cca = conActions[conIndex];

          if (cca instanceof CreateConstraintConstantAction)
          {

          This code looks through the constraint information that the parser placed into the tableElementList,
          as I described above. As it looks through the list, each constraint action in the list may be one
          of several types of constraints:

          • primary key constraint
          • foreign key constraint (REFERENCES ...)
          • check constraint (CHECK ...)

          For the purposes of this particular patch, we are only interested in primary key constraints,
          so we look at each constraint action in the list to see if it is a primary key constraint:

          int constraintType = cca.getConstraintType();
          if (constraintType == DataDictionary.PRIMARYKEY_CONSTRAINT)
          {

          If we find such a constraint, then we know that this ALTER TABLE statement is adding a new
          PRIMARY KEY constraint, so we want to check to see if the table already has such a constraint,
          in which case we want to reject this statement because a table can only have 1 PK constraint:

          ConstraintDescriptorList cdl =
          dd.getConstraintDescriptors(baseTable);

          if (cdl.getPrimaryKey() != null)

          { throw StandardException.newException( SQLState.LANG_ADD_PRIMARY_KEY_FAILED1, baseTable.getQualifiedName()); }

          Hopefully the proposed patch makes a little bit more sense now. Let me know what you think!

          Show
          Bryan Pendleton added a comment - Hi Eranda, Here's a few comments to hopefully explain how the attached patch works: During parsing, the parser processes the ALTER TABLE statement and breaks it down into pieces, then builds up a AlterTableNode instance to describe the statement. For an ALTER TABLE ADD ... statement, information about the column to be added, if any, and about the constraints that apply to that column, are stored in the table element list. This occurs at about lines 12622-12630 of sqlgrammar.jj, in the code: <ADD> ( tableElement = addColumnDefinition(tableElementList) tableElement = tableConstraintDefinition() ) { if (tableElement instanceof ColumnDefinitionNode) { //bug 5724 - auto increment columns not allowed in ALTER TABLE statement ColumnDefinitionNode cdn = (ColumnDefinitionNode) tableElement; if ( cdn.isAutoincrementColumn()) throw StandardException.newException(SQLState.LANG_ALTER_TABLE_AUTOINCREMENT_COLUMN_NOT_ALLOWED); } changeType [0] = DDLStatementNode.ADD_TYPE; tableElementList.addTableElement(tableElement); Later in the compilation of the ALTER TABLE statement, we call the makeConstantAction method in AlterTableNode.java, which in turn calls the prepConstantAction subroutine, at line 510 or so in AlterTableNode.java, which is the place where my proposed patch inserts some code. The first part of that new code looks like this: for (int conIndex = 0; conIndex < conActions.length; conIndex++) { ConstraintConstantAction cca = conActions [conIndex] ; if (cca instanceof CreateConstraintConstantAction) { This code looks through the constraint information that the parser placed into the tableElementList, as I described above. As it looks through the list, each constraint action in the list may be one of several types of constraints: primary key constraint foreign key constraint (REFERENCES ...) check constraint (CHECK ...) For the purposes of this particular patch, we are only interested in primary key constraints, so we look at each constraint action in the list to see if it is a primary key constraint: int constraintType = cca.getConstraintType(); if (constraintType == DataDictionary.PRIMARYKEY_CONSTRAINT) { If we find such a constraint, then we know that this ALTER TABLE statement is adding a new PRIMARY KEY constraint, so we want to check to see if the table already has such a constraint, in which case we want to reject this statement because a table can only have 1 PK constraint: ConstraintDescriptorList cdl = dd.getConstraintDescriptors(baseTable); if (cdl.getPrimaryKey() != null) { throw StandardException.newException( SQLState.LANG_ADD_PRIMARY_KEY_FAILED1, baseTable.getQualifiedName()); } Hopefully the proposed patch makes a little bit more sense now. Let me know what you think!
          Hide
          Eranda Sooriyabandara added a comment -

          Hi Bryan,
          Thanks for the explanation. I got the point why we must use that code.
          After patch using your patch file I ran the test and I got the error that
          tables was not there in the schema so I used the
          conn.commit() to the line-86 and the test was ran successfully in
          my platform. Here I am attaching the patch file after successful run.
          I think this ends the issue. Please let me know.
          I update derby-wiki about this issue, what I've done.
          Thanks
          Eranda

          Show
          Eranda Sooriyabandara added a comment - Hi Bryan, Thanks for the explanation. I got the point why we must use that code. After patch using your patch file I ran the test and I got the error that tables was not there in the schema so I used the conn.commit() to the line-86 and the test was ran successfully in my platform. Here I am attaching the patch file after successful run. I think this ends the issue. Please let me know. I update derby-wiki about this issue, what I've done. Thanks Eranda
          Hide
          Bryan Pendleton added a comment -

          Thanks for the revised patch, Eranda!

          I ran a complete set of regression tests, and committed the fix, as revision 795459.

          I decided not to remove the THROWASSERT from Heap.java at this time. I agree with Knut's observation that the THROWASSERT was not adding much value, but it also seemed that removing it was not essential to the patch, so in the interests of making a small, focused patch, I simply included the changes to AlterTableNode and AlterTableTest.

          I believe that the work on this issue is complete.

          Show
          Bryan Pendleton added a comment - Thanks for the revised patch, Eranda! I ran a complete set of regression tests, and committed the fix, as revision 795459. I decided not to remove the THROWASSERT from Heap.java at this time. I agree with Knut's observation that the THROWASSERT was not adding much value, but it also seemed that removing it was not essential to the patch, so in the interests of making a small, focused patch, I simply included the changes to AlterTableNode and AlterTableTest. I believe that the work on this issue is complete.
          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 to 10.5.

          Show
          Mike Matrigali added a comment - working on backporting this to 10.5.
          Hide
          Mike Matrigali added a comment -

          backported fix #795459 from trunk to 10.5 branch. In order to get the testcase for this issue also backported, needed to backport the AlterTableTest.java also.

          m105_jdk16:58>svn commit
          Sending .
          Sending java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java
          Sending java/testing/README.htm
          Deleting java/testing/org/apache/derbyTesting/functionTests/master/altertable.out
          Sending java/testing/org/apache/derbyTesting/functionTests/suites/derbylang.runall
          Adding java/testing/org/apache/derbyTesting/functionTests/tests/lang/AlterTableTest.java
          Sending java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java
          Deleting java/testing/org/apache/derbyTesting/functionTests/tests/lang/altertable.sql
          Deleting java/testing/org/apache/derbyTesting/functionTests/tests/lang/altertable_derby.properties
          Transmitting file data ....
          Committed revision 960222.

          Show
          Mike Matrigali added a comment - backported fix #795459 from trunk to 10.5 branch. In order to get the testcase for this issue also backported, needed to backport the AlterTableTest.java also. m105_jdk16:58>svn commit Sending . Sending java/engine/org/apache/derby/impl/sql/compile/AlterTableNode.java Sending java/testing/README.htm Deleting java/testing/org/apache/derbyTesting/functionTests/master/altertable.out Sending java/testing/org/apache/derbyTesting/functionTests/suites/derbylang.runall Adding java/testing/org/apache/derbyTesting/functionTests/tests/lang/AlterTableTest.java Sending java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java Deleting java/testing/org/apache/derbyTesting/functionTests/tests/lang/altertable.sql Deleting java/testing/org/apache/derbyTesting/functionTests/tests/lang/altertable_derby.properties Transmitting file data .... Committed revision 960222.
          Hide
          Mike Matrigali added a comment -

          resetting original issue owner.

          Show
          Mike Matrigali added a comment - resetting original issue owner.
          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 -

          [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.

            People

            • Assignee:
              Eranda Sooriyabandara
              Reporter:
              Bryan Pendleton
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development