You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Andreas Korneliussen (JIRA)" <de...@db.apache.org> on 2005/11/15 17:59:28 UTC

[jira] Commented: (DERBY-707) providing RowLocation for deleted+purged row to GenericConglomerateController causes nullpointerexception

    [ http://issues.apache.org/jira/browse/DERBY-707?page=comments#action_12357711 ] 

Andreas Korneliussen commented on DERBY-707:
--------------------------------------------

The attached files addresses this issue by checking the return value from open_conglom.latchPage(..) in GenericConglomerateController. Previously the return value has only been used in one of the four calls to open_conglom.latchPage(..) in this file, the patch also does the same for all three other places this is called (fetch, delete and replace).
Please review if this could be an appropriate patch for the problem reported. 

> providing RowLocation for deleted+purged row to GenericConglomerateController causes nullpointerexception
> ---------------------------------------------------------------------------------------------------------
>
>          Key: DERBY-707
>          URL: http://issues.apache.org/jira/browse/DERBY-707
>      Project: Derby
>         Type: Bug
>   Components: Store
>  Environment: Java 1.4
>     Reporter: Andreas Korneliussen
>     Assignee: Andreas Korneliussen
>     Priority: Minor
>  Attachments: DERBY-707.diff, DERBY-707.stat
>
> To provide scrollable updatable resultsets, we will store the RowLocation for every row in the ResultSet in a temporary table (backingstore hashtable).  The RowLocation is used to reposition the cursor before an update.
> The problem is when the row for the RowLocation has been deleted and purged by another transaction.  The update will then fail with a NullPointerException in GenericConglomerateController.replace(..):
> java.lang.NullPointerException
>         at org.apache.derby.impl.store.access.conglomerate.GenericConglomerateController.replace(GenericConglomerateController.java:465)
>         at org.apache.derby.impl.sql.execute.RowChangerImpl.updateRow(RowChangerImpl.java:516)
>         at org.apache.derby.impl.sql.execute.UpdateResultSet.collectAffectedRows(UpdateResultSet.java:577)
>         at org.apache.derby.impl.sql.execute.UpdateResultSet.open(UpdateResultSet.java:276)
>         at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:368)
>         at org.apache.derby.impl.jdbc.EmbedResultSet.updateRow(EmbedResultSet.java:3256) 
> It fails when checking if the row has been deleted (not purged) , because pos.current_page is null:
>         if (pos.current_page.isDeletedAtSlot(pos.current_slot))
>         {
>             ret_val = false;
>         }
> The proposed fix is to use the return value from open_conglom.latchPage(..).  If it returns false, the RowLocation is invalid (deleted+purged). 

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Re: [jira] Commented: (DERBY-707) providing RowLocation for deleted+purged row to GenericConglomerateController causes nullpointerexception

Posted by Andreas Korneliussen <An...@Sun.COM>.
Mike Matrigali wrote:
> This looks like a reasonable fix, it would be nice if you added a test
> case which showed the problem (to make sure it does not get broken again
> in the future).  If it were me I would add a short new test case to the
> T_AccessFactory tests in
> opensource/java/testing/org/apache/derbyTesting/unitTests/store.  I know
> you have a test case you can't check in as it is the feature you are
> working on.  If you are not interested in doing this, let me know.
> 

Hi,
I have now updated one of the testcases in T_AccessFactory to test this 
problem, and it reproduced the nullpointerexception (when running on 
unpatched derby)

I also noticed that after my patch, the method:

boolean fetch(
     RowLocation             loc,
     DataValueDescriptor[]   row,
     FormatableBitSet        validColumns )

is functionally equivalent to:

boolean fetch(
     RowLocation             loc,
     DataValueDescriptor[]   row,
     FormatableBitSet        validColumns,
     boolean                 waitForLock=true)


Previously, this bug had been fixed in the first method, not the other. 
To improve code-reuse, I would like to implement:

fetch(
     RowLocation             loc,
     DataValueDescriptor[]   row,
     FormatableBitSet        validColumns )

as:
{
    return fetch(loc, row, validColumns, true /* waitForLock */ );
}

.. unless there are any good reasons not to do it.

Andreas

Re: [jira] Commented: (DERBY-707) providing RowLocation for deleted+purged row to GenericConglomerateController causes nullpointerexception

Posted by Mike Matrigali <mi...@sbcglobal.net>.
This looks like a reasonable fix, it would be nice if you added a test
case which showed the problem (to make sure it does not get broken again
in the future).  If it were me I would add a short new test case to the
T_AccessFactory tests in
opensource/java/testing/org/apache/derbyTesting/unitTests/store.  I know
you have a test case you can't check in as it is the feature you are
working on.  If you are not interested in doing this, let me know.

Andreas Korneliussen (JIRA) wrote:

>     [ http://issues.apache.org/jira/browse/DERBY-707?page=comments#action_12357711 ] 
> 
> Andreas Korneliussen commented on DERBY-707:
> --------------------------------------------
> 
> The attached files addresses this issue by checking the return value from open_conglom.latchPage(..) in GenericConglomerateController. Previously the return value has only been used in one of the four calls to open_conglom.latchPage(..) in this file, the patch also does the same for all three other places this is called (fetch, delete and replace).
> Please review if this could be an appropriate patch for the problem reported. 
> 
> 
>>providing RowLocation for deleted+purged row to GenericConglomerateController causes nullpointerexception
>>---------------------------------------------------------------------------------------------------------
>>
>>         Key: DERBY-707
>>         URL: http://issues.apache.org/jira/browse/DERBY-707
>>     Project: Derby
>>        Type: Bug
>>  Components: Store
>> Environment: Java 1.4
>>    Reporter: Andreas Korneliussen
>>    Assignee: Andreas Korneliussen
>>    Priority: Minor
>> Attachments: DERBY-707.diff, DERBY-707.stat
>>
>>To provide scrollable updatable resultsets, we will store the RowLocation for every row in the ResultSet in a temporary table (backingstore hashtable).  The RowLocation is used to reposition the cursor before an update.
>>The problem is when the row for the RowLocation has been deleted and purged by another transaction.  The update will then fail with a NullPointerException in GenericConglomerateController.replace(..):
>>java.lang.NullPointerException
>>        at org.apache.derby.impl.store.access.conglomerate.GenericConglomerateController.replace(GenericConglomerateController.java:465)
>>        at org.apache.derby.impl.sql.execute.RowChangerImpl.updateRow(RowChangerImpl.java:516)
>>        at org.apache.derby.impl.sql.execute.UpdateResultSet.collectAffectedRows(UpdateResultSet.java:577)
>>        at org.apache.derby.impl.sql.execute.UpdateResultSet.open(UpdateResultSet.java:276)
>>        at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:368)
>>        at org.apache.derby.impl.jdbc.EmbedResultSet.updateRow(EmbedResultSet.java:3256) 
>>It fails when checking if the row has been deleted (not purged) , because pos.current_page is null:
>>        if (pos.current_page.isDeletedAtSlot(pos.current_slot))
>>        {
>>            ret_val = false;
>>        }
>>The proposed fix is to use the return value from open_conglom.latchPage(..).  If it returns false, the RowLocation is invalid (deleted+purged). 
> 
>