Re: BUG #1502: hash_seq_search might return removed entry

Bruce Momjian wrote:
> Thomas Hallgren wrote:
>
>>Tom Lane wrote:
>>
>>
>>>"Thomas" <thhal(at)mailblocks(dot)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