Обсуждение: SPI_finish and RegisterExprContextCallback

I'd like to write a C-function that returns a SETOF a complex type. This 
set is obtained from a query performed using an SPI cursor. I don't want 
to build the complete set in memory so I tried the following.

1. During the SRF_IS_FIRSTCALL phase, I do an SPI_connect and I create 
the cursor using SPI_prepare and SPI_cursor_open.
2. For each call, I obtain row(s) as needed using SPI_cursor_fetch. A 
row is copied before I return the tuple in a SRF_RETURN_NEXT
3. When I have no more rows, I close the cursor and issue a SPI_finish. 
Then I return SRF_RETURN DONE.

This works beautifully.

Now I'm trying to deal with scenarios where I never reach the end of the 
set because the evaluator doesn't need all rows. So I use 
RegisterExprContextCallback to register a callback and instead of doing 
an SPI_finish when the end is reached I attempt to do this in the 
callback. The callback is called OK but when it calls SPI_finish I get 
an illegal memory access signal.

How am I supposed to do this? Is it at all possible to stream the 
results of one query to another using a SETOF function?

Regards,
Thomas Hallgren

Thomas Hallgren <thhal@mailblocks.com> writes:
> .... The callback is called OK but when it calls SPI_finish I get 
> an illegal memory access signal.

From where?

Minimum respect for the time of your fellow hackers would suggest
including a gdb traceback in questions like this.
        regards, tom lane

Tom Lane wrote:

>From where?
>
>Minimum respect for the time of your fellow hackers would suggest
>including a gdb traceback in questions like this.
>  
>
My apologies. I'll do that next time. I was on a win32 system and the 
gdb that comes with the MinGW environment just doesn't do it for me (if 
anyone out there knows how to make the MinGW gdb work I'd very much 
appreciate any advice). I have a Linux box too though, so that's no excuse.

Anyway, I think I've narrowed the problem down a bit. And indeed, I 
think there is a somewhat serious limitation in the SPI layer. Here's 
what happens:

1. I call a function that does an SPI_connect, SPI_prepare, 
SPI_cursor_open, and finally attempts to do an SPI_cursor_fetch.
2. Since the SQL statement I'm executing contains a call to function 
returning SETOF, and since that function in turn accesses the database, 
it in turn will issue a SPI_connect in its SRF_IS_FIRSTCALL phase. It 
then returns its first row.
3. The SPI_cursor_fetch call in my outer function now fails with 
"improper call to spi_printtup" since it is asociated with the first 
SPI_connect and since the second SPI_connect has not reached it's 
matching SPI_finish yet.

I onclude that with the current implementation there's no way of 
achiving data "streaming" using SPI. When I say streaming, I mean a 
SETOF function that, one row at a time, delivers the result that it 
reads from a SPI_cursor. No matter what I do, short of building the 
whole set in memory, will result in unbalanced SPI_connect/SPI_finish 
calls. With reservations for me missing something painfully obvious of 
course.

Regards,
Thomas Hallgren

Thomas Hallgren <thhal@mailblocks.com> writes:
> 1. I call a function that does an SPI_connect, SPI_prepare, 
> SPI_cursor_open, and finally attempts to do an SPI_cursor_fetch.
> 2. Since the SQL statement I'm executing contains a call to function 
> returning SETOF, and since that function in turn accesses the database, 
> it in turn will issue a SPI_connect in its SRF_IS_FIRSTCALL phase. It 
> then returns its first row.

You're right, you can't just return from that inner function while
leaving its SPI connection open.

It might be interesting to redesign SPI around the notion of independent
"connection objects" rather than necessarily having a stack of 'em.
I think that could be made to work ... but not while preserving the
existing SPI API.  I'm hesitant to break a ton of user-written code for
a feature that only one person has needed :-(
        regards, tom lane

Tom,

>You're right, you can't just return from that inner function while
>leaving its SPI connection open.
>
>It might be interesting to redesign SPI around the notion of independent
>"connection objects" rather than necessarily having a stack of 'em.
>  
>
I made the same reflection looking at the SPI code. It would be nice if 
something corresponding to _SPI_current could be passed around.

>I think that could be made to work ... but not while preserving the
>existing SPI API.
>
I'm not so sure you'd have to. A public API that can disable stack 
handling and instead use something similar to MemoryContextSwitchTo but 
for an _SPI_current like structure would perhaps be sufficient?

>  I'm hesitant to break a ton of user-written code for
>a feature that only one person has needed :-(
>  
>
It is a fairly serious design flaw IMHO. I discovered it and so far no 
one else has complained. That's true for all flaws at first.

Regards,
Thomas Hallgren

I found another piece of information that might be of interest. This is 
related to nested calls and the ExprContextCallback but not related to 
nested active cursors.

AtCommitPortals (portalmem.c)  iterates over the entries in the 
PortalHashTable. This causes a chain of calls that sometimes reach an 
ExprContextCallback. If that callback issues a succesfull 
SPI_cursor_close some problems might lay ahead. As the AcCommitPortals 
iteration continues, it sometimes encounter a deleted portal and elogs 
with an error stating "trying to delete portal name that does not exist".

Regards,
Thomas Hallgren

Thomas Hallgren <thhal@mailblocks.com> writes:
> AtCommitPortals (portalmem.c)  iterates over the entries in the 
> PortalHashTable. This causes a chain of calls that sometimes reach an 
> ExprContextCallback. If that callback issues a succesfull 
> SPI_cursor_close some problems might lay ahead. As the AcCommitPortals 
> iteration continues, it sometimes encounter a deleted portal and elogs 
> with an error stating "trying to delete portal name that does not exist".

The comment for AtCommit_Portals points out that there are risks of this
sort, but I don't think you've described it properly.  The
SPI_cursor_close operation is probably failing not succeeding, because
AtCommit_Portals will never find an already-deleted portal ---
hash_seq_search() shouldn't return any already-dropped entries.
        regards, tom lane

Tom Lane wrote:

>The comment for AtCommit_Portals points out that there are risks of this
>sort, but I don't think you've described it properly.  The
>SPI_cursor_close operation is probably failing not succeeding, because
>AtCommit_Portals will never find an already-deleted portal ---
>hash_seq_search() shouldn't return any already-dropped entries.
>  
>
The hash_seq_search keeps track of what element that it should return 
next when it peruses a bucket. Removing that element from the table 
won't change anything since the HASH_SEQ_STATUS remains unaffected. It 
still holds onto that element and hence, will return it on next iteration.

This should be considered a bug I think.

Regards,
Thomas Hallgren