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 "Knut Anders Hatlen (JIRA)" <ji...@apache.org> on 2010/06/01 21:28:38 UTC

[jira] Created: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
----------------------------------------------------------------

                 Key: DERBY-4685
                 URL: https://issues.apache.org/jira/browse/DERBY-4685
             Project: Derby
          Issue Type: Improvement
          Components: Store
    Affects Versions: 10.7.0.0
            Reporter: Knut Anders Hatlen
            Priority: Minor


OpenConglomerate.lockPositionForWrite() contains this code twice:

            if (!waitForLock)
            {
                // throw lock timeout error.
                throw StandardException.newException(SQLState.LOCK_TIMEOUT);
            }

The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)

Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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


[jira] Updated: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen updated DERBY-4685:
--------------------------------------

    Issue & fix info: [Patch Available]

Regression tests ran cleanly with the #2 patch.

> Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
> ----------------------------------------------------------------
>
>                 Key: DERBY-4685
>                 URL: https://issues.apache.org/jira/browse/DERBY-4685
>             Project: Derby
>          Issue Type: Improvement
>          Components: Store
>    Affects Versions: 10.7.1.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d4685-2.diff, d4685.diff
>
>
> OpenConglomerate.lockPositionForWrite() contains this code twice:
>             if (!waitForLock)
>             {
>                 // throw lock timeout error.
>                 throw StandardException.newException(SQLState.LOCK_TIMEOUT);
>             }
> The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)
> Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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


[jira] Commented: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12926244#action_12926244 ] 

Kristian Waagan commented on DERBY-4685:
----------------------------------------

Looks good to me, +1 to commit.

> Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
> ----------------------------------------------------------------
>
>                 Key: DERBY-4685
>                 URL: https://issues.apache.org/jira/browse/DERBY-4685
>             Project: Derby
>          Issue Type: Improvement
>          Components: Store
>    Affects Versions: 10.7.1.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d4685-2.diff, d4685.diff
>
>
> OpenConglomerate.lockPositionForWrite() contains this code twice:
>             if (!waitForLock)
>             {
>                 // throw lock timeout error.
>                 throw StandardException.newException(SQLState.LOCK_TIMEOUT);
>             }
> The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)
> Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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


[jira] Updated: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen updated DERBY-4685:
--------------------------------------

    Issue & fix info: [Patch Available]
            Assignee: Knut Anders Hatlen

> Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
> ----------------------------------------------------------------
>
>                 Key: DERBY-4685
>                 URL: https://issues.apache.org/jira/browse/DERBY-4685
>             Project: Derby
>          Issue Type: Improvement
>          Components: Store
>    Affects Versions: 10.7.1.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d4685.diff
>
>
> OpenConglomerate.lockPositionForWrite() contains this code twice:
>             if (!waitForLock)
>             {
>                 // throw lock timeout error.
>                 throw StandardException.newException(SQLState.LOCK_TIMEOUT);
>             }
> The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)
> Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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


[jira] Updated: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen updated DERBY-4685:
--------------------------------------

    Attachment: d4685.diff

Here's a patch that removes the first check for !waitForLock, so that the latch release code isn't hidden. The second identical check right after latch release will make sure that the exception is still raised.

All the regression tests ran cleanly with the patch.

> Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
> ----------------------------------------------------------------
>
>                 Key: DERBY-4685
>                 URL: https://issues.apache.org/jira/browse/DERBY-4685
>             Project: Derby
>          Issue Type: Improvement
>          Components: Store
>    Affects Versions: 10.7.1.0
>            Reporter: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d4685.diff
>
>
> OpenConglomerate.lockPositionForWrite() contains this code twice:
>             if (!waitForLock)
>             {
>                 // throw lock timeout error.
>                 throw StandardException.newException(SQLState.LOCK_TIMEOUT);
>             }
> The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)
> Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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


[jira] Closed: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen closed DERBY-4685.
-------------------------------------

          Resolution: Fixed
       Fix Version/s: 10.7.1.0
    Issue & fix info:   (was: [Patch Available])

Thanks, Kristian.
Committed revision 1028712.

> Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
> ----------------------------------------------------------------
>
>                 Key: DERBY-4685
>                 URL: https://issues.apache.org/jira/browse/DERBY-4685
>             Project: Derby
>          Issue Type: Improvement
>          Components: Store
>    Affects Versions: 10.7.1.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>             Fix For: 10.7.1.0
>
>         Attachments: d4685-2.diff, d4685.diff
>
>
> OpenConglomerate.lockPositionForWrite() contains this code twice:
>             if (!waitForLock)
>             {
>                 // throw lock timeout error.
>                 throw StandardException.newException(SQLState.LOCK_TIMEOUT);
>             }
> The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)
> Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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


[jira] Updated: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen updated DERBY-4685:
--------------------------------------

    Issue & fix info:   (was: [Patch Available])

Committed revision 1028639.

> Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
> ----------------------------------------------------------------
>
>                 Key: DERBY-4685
>                 URL: https://issues.apache.org/jira/browse/DERBY-4685
>             Project: Derby
>          Issue Type: Improvement
>          Components: Store
>    Affects Versions: 10.7.1.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d4685.diff
>
>
> OpenConglomerate.lockPositionForWrite() contains this code twice:
>             if (!waitForLock)
>             {
>                 // throw lock timeout error.
>                 throw StandardException.newException(SQLState.LOCK_TIMEOUT);
>             }
> The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)
> Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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


[jira] Updated: (DERBY-4685) Dead/unreachable code in OpenConglomerate.lockPositionForWrite()

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen updated DERBY-4685:
--------------------------------------

    Attachment: d4685-2.diff

Attaching a patch (#2) that addresses the other issue mentioned in the description. The patch removes the second parameter (forInsert) in lockPositionForWrite() because it is always false in the current code, and because the method is not believed to work for inserts anyhow.

> Dead/unreachable code in OpenConglomerate.lockPositionForWrite()
> ----------------------------------------------------------------
>
>                 Key: DERBY-4685
>                 URL: https://issues.apache.org/jira/browse/DERBY-4685
>             Project: Derby
>          Issue Type: Improvement
>          Components: Store
>    Affects Versions: 10.7.1.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d4685-2.diff, d4685.diff
>
>
> OpenConglomerate.lockPositionForWrite() contains this code twice:
>             if (!waitForLock)
>             {
>                 // throw lock timeout error.
>                 throw StandardException.newException(SQLState.LOCK_TIMEOUT);
>             }
> The second occurrence of this code can never end up throwing an exception, since waitForLock is guaranteed to be true there because of the identical check a couple of lines above. (Judging by the similar method lockPositionForRead(), it is probably the first check that should be removed, so that the latch on the page is released before the exception is thrown.)
> Also, the method is always called with forInsert==false, so the forInsert parameter can be removed. (I also suspect that the method doesn't work correctly if ever used in an insert operation, since it calls latchPage(RowPosition) which will unlatch the page if the row isn't found on the page, and I assume that a row that is about to be inserted does not exist yet.)

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