The following bug has been logged online:

Bug reference: 1502
Logged by: Thomas
Email address: thhal@mailblocks.com
PostgreSQL version: 8.0.1
Operating system: N/A
Description: hash_seq_search might return removed entry
Details:

The hash_seq_search keeps track of what element that it should return next
in a HASH_SEQ_STATUS struct when it peruses a bucket. Removing that element
from the table won't change anything since the struct remains unaffected. It
still holds onto that element and hence, will return it on next iteration.

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

"Thomas" <thhal@mailblocks.com> writes:
> The hash_seq_search keeps track of what element that it should return next
> in a HASH_SEQ_STATUS struct when it peruses a bucket. Removing that element
> from the table won't change anything since the struct remains unaffected. It
> still holds onto that element and hence, will return it on next iteration.

This isn't a bug; it's the designed way for it to work. It's up to
callers to avoid causing a problem.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

Tom Lane wrote:

>"Thomas" <thhal@mailblocks.com> writes:
>
>
>>The hash_seq_search keeps track of what element that it should return next
>>in a HASH_SEQ_STATUS struct when it peruses a bucket. Removing that element
>>from the table won't change anything since the struct remains unaffected. It
>>still holds onto that element and hence, will return it on next iteration.
>>
>>
>
>This isn't a bug; it's the designed way for it to work. It's up to
>callers to avoid causing a problem.
>
>
This report origins from the hackers thread "SPI_finish and
RegisterExprContextCallback" where you indicated that my report did not
properly describe a problem. Since my report was correct and since you
stated that "hash_seq_search() shouldn't return any already-dropped
entries." I was led me to believe that it was *not* designed to do that.

Anyway, the AtCommit_Portals doesn't avoid this problem and the end
result for me is the same regardless of where the error is. I can't drop
portals using a ExprContextCallback and I'd like to know the best way to
fix it. I can contribute a patch but I want you to decide what needs to
be fixed.

Regards,
Thomas Hallgren



---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

Thomas Hallgren wrote:
> Tom Lane wrote:
>
> >"Thomas" <thhal@mailblocks.com> writes:
> >
> >
> >>The hash_seq_search keeps track of what element that it should return next
> >>in a HASH_SEQ_STATUS struct when it peruses a bucket. Removing that element
> >>from the table won't change anything since the struct remains unaffected. It
> >>still holds onto that element and hence, will return it on next iteration.
> >>
> >>
> >
> >This isn't a bug; it's the designed way for it to work. It's up to
> >callers to avoid causing a problem.
> >
> >
> This report origins from the hackers thread "SPI_finish and
> RegisterExprContextCallback" where you indicated that my report did not
> properly describe a problem. Since my report was correct and since you
> stated that "hash_seq_search() shouldn't return any already-dropped
> entries." I was led me to believe that it was *not* designed to do that.
>
> Anyway, the AtCommit_Portals doesn't avoid this problem and the end
> result for me is the same regardless of where the error is. I can't drop
> portals using a ExprContextCallback and I'd like to know the best way to
> fix it. I can contribute a patch but I want you to decide what needs to
> be fixed.

Does this need a C comment addition?

--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

Bruce Momjian wrote:
> Thomas Hallgren wrote:
>
>>Tom Lane wrote:
>>
>>
>>>"Thomas" <thhal@mailblocks.com> writes:
>>>
>>>
>>>
>>>>The hash_seq_search keeps track of what element that it should return next
>>>>in a HASH_SEQ_STATUS struct when it peruses a bucket. Removing that element
>>>
>>>>from the table won't change anything since the struct remains unaffected. It
>>>
>>>>still holds onto that element and hence, will return it on next iteration.
>>>>
>>>>
>>>
>>>This isn't a bug; it's the designed way for it to work. It's up to
>>>callers to avoid causing a problem.
>>>
>>>
>>
>>This report origins from the hackers thread "SPI_finish and
>>RegisterExprContextCallback" where you indicated that my report did not
>>properly describe a problem. Since my report was correct and since you
>>stated that "hash_seq_search() shouldn't return any already-dropped
>>entries." I was led me to believe that it was *not* designed to do that.
>>
>>Anyway, the AtCommit_Portals doesn't avoid this problem and the end
>>result for me is the same regardless of where the error is. I can't drop
>>portals using a ExprContextCallback and I'd like to know the best way to
>>fix it. I can contribute a patch but I want you to decide what needs to
>>be fixed.
>
>
> Does this need a C comment addition?
>
I'm thinking of providing a remedy for my problem that does the following.

1. The AtCommit_Portals will start with building a fixed size array of
pointers to portals that is the result of an iteration of the hash table.

2. Next, it will iterate over this array and for each pointer check if
that pointer is NULL. If not, do what's done during the iteration today.

3. The method that deletes a portal from the hash-table will also
sequentially scan the pointer array and set the pointer to NULL if found.

The remedy is based on the fact that it's unlikely that you have a large
amount of portals in the hash table.

Would such a patch be accepted?

Regards,
Thomas Hallgren



---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

Thomas Hallgren <thhal@mailblocks.com> writes:
> Would such a patch be accepted?

Seems like a brute-force solution. I'd look first at whether
AtCommit_Portals could just restart its hashtable scan after
each deletion; if that seems too inefficient, modify the hash
table entries themselves to carry a "portal already deleted"
flag.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

Tom Lane wrote:

>Thomas Hallgren <thhal@mailblocks.com> writes:
>
>
>>Would such a patch be accepted?
>>
>>
>
>Seems like a brute-force solution. I'd look first at whether
>AtCommit_Portals could just restart its hashtable scan after
>each deletion; if that seems too inefficient, modify the hash
>table entries themselves to carry a "portal already deleted"
>flag.
>
>
Yes, a static flag indicating that a deletion has occured will work fine
since all portals that has not been perused but not dropped now has an
InvalidSubTransactionId. I'll do it that way then.

Regards,
Thomas Hallgren



---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org