Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

Started by Andy Fanover 5 years ago8 messages
#1Andy Fan
zhihui.fan1213@gmail.com

I have a user case like this:

rs = prepared_stmt.execute(1);
while(rs.next())
{
// do something with the result and commit the transaction.
conn.commit();
}

The driver used the extended protocol in this case. It works like this: 1).
Parse ->
PreparedStmt. 2). Bind -> Bind the prepared stmt with a Portal, no chance
to
set the CURSOR_OPT_HOLD option. 3). Execute. 4). Commit - the portal was
dropped at this stage. 5). when fetching the next batch of results, we get
the error
"Portal doesn't exist"

There are several methods we can work around this, but no one is perfect.
1.run the prepared stmt in a dedicated connection. (The number of
connection will
doubled)
2. use the with hold cursor. It doesn't support any bind parameter, so we
have
to create a cursor for each dedicated id.
3. don't commit the transaction. -- long transaction with many rows locked.

I have several questions about this case:
1. How about filling a cursorOptions information in bind protocol? then we
can
set the portal->cursorOptions accordingly? if so, how to be compatible
with the
old driver usually?
2. Currently I want to add a new GUC parameter, if set it to true, server
will
create a holdable portal, or else nothing changed. Then let the user set
it to true in the above case and reset it to false afterward. Is there any
issue
with this method?

--
Best Regards
Andy Fan

#2Andy Fan
zhihui.fan1213@gmail.com
In reply to: Andy Fan (#1)
Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

2. Currently I want to add a new GUC parameter, if set it to true, server
will
create a holdable portal, or else nothing changed. Then let the user set
it to true in the above case and reset it to false afterward. Is there
any issue
with this method?

I forget to say in this case, the user has to drop the holdable
portal explicitly.

--
Best Regards
Andy Fan

#3Andy Fan
zhihui.fan1213@gmail.com
In reply to: Andy Fan (#2)
Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

On Mon, Jul 27, 2020 at 11:57 AM Andy Fan <zhihui.fan1213@gmail.com> wrote:

2. Currently I want to add a new GUC parameter, if set it to true, server
will
create a holdable portal, or else nothing changed. Then let the user set
it to true in the above case and reset it to false afterward. Is there
any issue
with this method?

I forget to say in this case, the user has to drop the holdable
portal explicitly.

After some days's hack and testing, I found more issues to support the
following case

rs = prepared_stmt.execute(1);
while(rs.next())
{
// do something with the result (mainly DML )
conn.commit(); or conn.rollback();

// commit / rollback to avoid the long lock holding.
}

The holdable portal is still be dropped in transaction aborted/rollbacked
case since
the HoldPortal doesn't happens before that and "abort/rollabck" means
something
wrong so it is risk to hold it again. What I did to fix this issue is
HoldPortal just after
we define a Holdable portal. However, that's bad for performance.
Originally, we just
needed to scan the result when needed, now we have to hold all the results
and then fetch
and the data one by one.

The above user case looks reasonable to me IMO, I would say it is kind of
"tech debt"
in postgres. To support this completely, looks we have to decouple the
snapshot/locking
management with transaction? If so, it looks like a huge change. I wonder
if anybody
tried to resolve this issue and where do we get to that point?

--
Best Regards
Andy Fan

#4Dave Cramer
davecramer@postgres.rocks
In reply to: Andy Fan (#3)
Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

On Tue, 11 Aug 2020 at 22:33, Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Mon, Jul 27, 2020 at 11:57 AM Andy Fan <zhihui.fan1213@gmail.com>
wrote:

2. Currently I want to add a new GUC parameter, if set it to true,
server will
create a holdable portal, or else nothing changed. Then let the user
set
it to true in the above case and reset it to false afterward. Is there
any issue
with this method?

I forget to say in this case, the user has to drop the holdable
portal explicitly.

After some days's hack and testing, I found more issues to support the
following case

rs = prepared_stmt.execute(1);
while(rs.next())
{
// do something with the result (mainly DML )
conn.commit(); or conn.rollback();

// commit / rollback to avoid the long lock holding.
}

The holdable portal is still be dropped in transaction aborted/rollbacked
case since
the HoldPortal doesn't happens before that and "abort/rollabck" means
something
wrong so it is risk to hold it again. What I did to fix this issue is
HoldPortal just after
we define a Holdable portal. However, that's bad for performance.
Originally, we just
needed to scan the result when needed, now we have to hold all the results
and then fetch
and the data one by one.

The above user case looks reasonable to me IMO, I would say it is kind of
"tech debt"
in postgres. To support this completely, looks we have to decouple the
snapshot/locking
management with transaction? If so, it looks like a huge change. I wonder
if anybody
tried to resolve this issue and where do we get to that point?

--
Best Regards
Andy Fan

I think if you set the fetch size the driver will use a named cursor and
this should work

Dave Cramer
www.postgres.rocks

#5Andy Fan
zhihui.fan1213@gmail.com
In reply to: Dave Cramer (#4)
Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

On Wed, Aug 12, 2020 at 5:54 PM Dave Cramer <davecramer@postgres.rocks>
wrote:

On Tue, 11 Aug 2020 at 22:33, Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Mon, Jul 27, 2020 at 11:57 AM Andy Fan <zhihui.fan1213@gmail.com>
wrote:

2. Currently I want to add a new GUC parameter, if set it to true,
server will
create a holdable portal, or else nothing changed. Then let the user
set
it to true in the above case and reset it to false afterward. Is there
any issue
with this method?

I forget to say in this case, the user has to drop the holdable
portal explicitly.

After some days's hack and testing, I found more issues to support the
following case

rs = prepared_stmt.execute(1);
while(rs.next())
{
// do something with the result (mainly DML )
conn.commit(); or conn.rollback();

// commit / rollback to avoid the long lock holding.
}

The holdable portal is still be dropped in transaction aborted/rollbacked
case since
the HoldPortal doesn't happens before that and "abort/rollabck" means
something
wrong so it is risk to hold it again. What I did to fix this issue is
HoldPortal just after
we define a Holdable portal. However, that's bad for performance.
Originally, we just
needed to scan the result when needed, now we have to hold all the
results and then fetch
and the data one by one.

The above user case looks reasonable to me IMO, I would say it is kind
of "tech debt"
in postgres. To support this completely, looks we have to decouple the
snapshot/locking
management with transaction? If so, it looks like a huge change. I wonder
if anybody
tried to resolve this issue and where do we get to that point?

--
Best Regards
Andy Fan

I think if you set the fetch size the driver will use a named cursor and
this should work

I knew this point before working on that, but I heard from my customer that
the size
would be pretty big, so I gave up on this idea (too early). However,
after working on
a Holdable solution, I see there is very little difference between caching
the result
on the server or client. If the drivers can use the tempfile as an extra
store, then
things will be better than the server. Or else, things will be still
complex. Thanks
for your reminder!

--
Best Regards
Andy Fan

#6Andy Fan
zhihui.fan1213@gmail.com
In reply to: Andy Fan (#5)
Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

On Wed, Aug 12, 2020 at 8:11 PM Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Wed, Aug 12, 2020 at 5:54 PM Dave Cramer <davecramer@postgres.rocks>
wrote:

On Tue, 11 Aug 2020 at 22:33, Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Mon, Jul 27, 2020 at 11:57 AM Andy Fan <zhihui.fan1213@gmail.com>
wrote:

2. Currently I want to add a new GUC parameter, if set it to true,
server will
create a holdable portal, or else nothing changed. Then let the user
set
it to true in the above case and reset it to false afterward. Is
there any issue
with this method?

I forget to say in this case, the user has to drop the holdable
portal explicitly.

After some days's hack and testing, I found more issues to support the
following case

rs = prepared_stmt.execute(1);
while(rs.next())
{
// do something with the result (mainly DML )
conn.commit(); or conn.rollback();

// commit / rollback to avoid the long lock holding.
}

The holdable portal is still be dropped in transaction
aborted/rollbacked case since
the HoldPortal doesn't happens before that and "abort/rollabck" means
something
wrong so it is risk to hold it again. What I did to fix this issue is
HoldPortal just after
we define a Holdable portal. However, that's bad for performance.
Originally, we just
needed to scan the result when needed, now we have to hold all the
results and then fetch
and the data one by one.

The above user case looks reasonable to me IMO, I would say it is kind
of "tech debt"
in postgres. To support this completely, looks we have to decouple the
snapshot/locking
management with transaction? If so, it looks like a huge change. I
wonder if anybody
tried to resolve this issue and where do we get to that point?

--
Best Regards
Andy Fan

I think if you set the fetch size the driver will use a named cursor and
this should work

If the drivers can use the tempfile as an extra store, then things will be
better than the server.

Maybe not much better, just the same as each other. Both need to
store all of them first and fetch them from the temp store again.

--
Best Regards
Andy Fan

#7Dave Cramer
davecramer@postgres.rocks
In reply to: Andy Fan (#6)
Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

On Wed, 12 Aug 2020 at 08:14, Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Wed, Aug 12, 2020 at 8:11 PM Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Wed, Aug 12, 2020 at 5:54 PM Dave Cramer <davecramer@postgres.rocks>
wrote:

On Tue, 11 Aug 2020 at 22:33, Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Mon, Jul 27, 2020 at 11:57 AM Andy Fan <zhihui.fan1213@gmail.com>
wrote:

2. Currently I want to add a new GUC parameter, if set it to true,
server will
create a holdable portal, or else nothing changed. Then let the user
set
it to true in the above case and reset it to false afterward. Is
there any issue
with this method?

I forget to say in this case, the user has to drop the holdable
portal explicitly.

After some days's hack and testing, I found more issues to support the
following case

rs = prepared_stmt.execute(1);
while(rs.next())
{
// do something with the result (mainly DML )
conn.commit(); or conn.rollback();

// commit / rollback to avoid the long lock holding.
}

The holdable portal is still be dropped in transaction
aborted/rollbacked case since
the HoldPortal doesn't happens before that and "abort/rollabck" means
something
wrong so it is risk to hold it again. What I did to fix this issue is
HoldPortal just after
we define a Holdable portal. However, that's bad for performance.
Originally, we just
needed to scan the result when needed, now we have to hold all the
results and then fetch
and the data one by one.

The above user case looks reasonable to me IMO, I would say it is kind
of "tech debt"
in postgres. To support this completely, looks we have to decouple the
snapshot/locking
management with transaction? If so, it looks like a huge change. I
wonder if anybody
tried to resolve this issue and where do we get to that point?

--
Best Regards
Andy Fan

I think if you set the fetch size the driver will use a named cursor and
this should work

If the drivers can use the tempfile as an extra store, then things will
be better than the server.

Maybe not much better, just the same as each other. Both need to
store all of them first and fetch them from the temp store again.

Ya I thought about this after I answered it. If you have a resultset that
you requested in a transaction and then you commit the transaction I think
it is reasonable to expect that the resultset is no longer valid.

Dave Cramer
www.postgres.rocks

Show quoted text
#8Andy Fan
zhihui.fan1213@gmail.com
In reply to: Dave Cramer (#7)
Re: Allows Extend Protocol support CURSOR_OPT_HOLD with prepared stmt.

On Wed, Aug 12, 2020 at 8:21 PM Dave Cramer <davecramer@postgres.rocks>
wrote:

On Wed, 12 Aug 2020 at 08:14, Andy Fan <zhihui.fan1213@gmail.com> wrote:

On Wed, Aug 12, 2020 at 8:11 PM Andy Fan <zhihui.fan1213@gmail.com>
wrote:

On Wed, Aug 12, 2020 at 5:54 PM Dave Cramer <davecramer@postgres.rocks>
wrote:

On Tue, 11 Aug 2020 at 22:33, Andy Fan <zhihui.fan1213@gmail.com>
wrote:

On Mon, Jul 27, 2020 at 11:57 AM Andy Fan <zhihui.fan1213@gmail.com>
wrote:

2. Currently I want to add a new GUC parameter, if set it to true,
server will
create a holdable portal, or else nothing changed. Then let the
user set
it to true in the above case and reset it to false afterward. Is
there any issue
with this method?

I forget to say in this case, the user has to drop the holdable
portal explicitly.

After some days's hack and testing, I found more issues to support the
following case

rs = prepared_stmt.execute(1);
while(rs.next())
{
// do something with the result (mainly DML )
conn.commit(); or conn.rollback();

// commit / rollback to avoid the long lock holding.
}

The holdable portal is still be dropped in transaction
aborted/rollbacked case since
the HoldPortal doesn't happens before that and "abort/rollabck" means
something
wrong so it is risk to hold it again. What I did to fix this issue is
HoldPortal just after
we define a Holdable portal. However, that's bad for performance.
Originally, we just
needed to scan the result when needed, now we have to hold all the
results and then fetch
and the data one by one.

The above user case looks reasonable to me IMO, I would say it is
kind of "tech debt"
in postgres. To support this completely, looks we have to decouple
the snapshot/locking
management with transaction? If so, it looks like a huge change. I
wonder if anybody
tried to resolve this issue and where do we get to that point?

--
Best Regards
Andy Fan

I think if you set the fetch size the driver will use a named cursor
and this should work

If the drivers can use the tempfile as an extra store, then things will
be better than the server.

Maybe not much better, just the same as each other. Both need to
store all of them first and fetch them from the temp store again.

Ya I thought about this after I answered it. If you have a resultset that
you requested in a transaction and then you commit the transaction I think
it is reasonable to expect that the resultset is no longer valid.

I checked JDBC, the resultset only uses memory to cache the resultset.
so we can't set an inf+ fetch size with the hope that the client's
resultset
can cache all of them for us.

Basically I will use my above hack.

--
Best Regards
Andy Fan