db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bryan Pendleton (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-4244) ALTER TABLE Sanity ASSERT in add column with autocommit off
Date Sun, 12 Jul 2009 18:27:15 GMT

    [ https://issues.apache.org/jira/browse/DERBY-4244?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12730127#action_12730127
] 

Bryan Pendleton commented on DERBY-4244:
----------------------------------------

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!


> ALTER TABLE Sanity ASSERT in add column with autocommit off
> -----------------------------------------------------------
>
>                 Key: DERBY-4244
>                 URL: https://issues.apache.org/jira/browse/DERBY-4244
>             Project: Derby
>          Issue Type: Bug
>          Components: SQL
>    Affects Versions: 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.0.0
>            Reporter: Bryan Pendleton
>            Assignee: Eranda Sooriyabandara
>            Priority: Minor
>         Attachments: checkAtCompileTime.diff
>
>
> 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)));
>             }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message