CommandStatus from insert returning when using a portal.

On Wed, Jul 12, 2023 at 1:03 PM Dave Cramer <davecramer@gmail.com> wrote:

INSERT INTO test_table (cnt) VALUES (1), (2) RETURNING id

if a portal is used to get the results then the CommandStatus

IIUC the portal is not optional if you including the RETURNING clause.

There is no CommandStatus message in the protocol, the desired information
is part of the command tag returned in the CommandComplete message. You
get that at the end of the command, which has been defined as when any
portal produced by the command has been fully executed.

You probably should add your desire to the Version 4 protocol ToDo on the
wiki.

https://wiki.postgresql.org/wiki/Todo#Wire_Protocol_Changes_.2F_v4_Protocol

If that ever becomes an active project working through the details of that
list for desirability and feasibility would be the first thing to happen.

David J.

On Wed, Jul 12, 2023 at 1:03 PM Dave Cramer <davecramer@gmail.com> wrote:

With a simple insert such as

INSERT INTO test_table (cnt) VALUES (1), (2) RETURNING id

if a portal is used to get the results then the CommandStatus is not returned on the execute only when the portal is closed. After looking at this more it is really after all of the data is read which is consistent if you don't use a portal, however it would be much more useful if we received the CommandStatus after the insert was completed and before the data

Obviously I am biased by the JDBC API which would like to have

PreparedStatement.execute() return the number of rows inserted without having to wait to read all of the rows returned

I believe if RETURNING clause is use, the protocol-level behaviour of
INSERT is expected to match that of SELECT. If the SELECT command
behaves like that (resultset followed by CommandStatus), then I'd say
the INSERT RETURNING is behaving as expected.

Best regards,
Gurjeet
http://Gurje.et

Dave Cramer

On Wed, 12 Jul 2023 at 16:31, David G. Johnston <david.g.johnston@gmail.com>
wrote:

On Wed, Jul 12, 2023 at 1:03 PM Dave Cramer <davecramer@gmail.com> wrote:

INSERT INTO test_table (cnt) VALUES (1), (2) RETURNING id

if a portal is used to get the results then the CommandStatus

IIUC the portal is not optional if you including the RETURNING clause.

From my testing it isn't required.

There is no CommandStatus message in the protocol, the desired information
is part of the command tag returned in the CommandComplete message. You
get that at the end of the command, which has been defined as when any
portal produced by the command has been fully executed.

I could argue that the insert is fully completed whether I read the data or
not.

You probably should add your desire to the Version 4 protocol ToDo on the
wiki.

https://wiki.postgresql.org/wiki/Todo#Wire_Protocol_Changes_.2F_v4_Protocol

thx, will do.

Dave

Dave Cramer <davecramer@gmail.com> writes:

Obviously I am biased by the JDBC API which would like to have
PreparedStatement.execute() return the number of rows inserted
without having to wait to read all of the rows returned

Umm ... you do realize that we return the rows on-the-fly?
The server does not know how many rows got inserted/returned
until it's run the query to completion, at which point all
the data has already been sent to the client. There isn't
any way to return the rowcount before the data, and it wouldn't
be some trivial protocol adjustment to make that work differently.
(What it *would* be is expensive, because we'd have to store
those rows somewhere.)

regards, tom lane

On Wed, 12 Jul 2023 at 17:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dave Cramer <davecramer@gmail.com> writes:

Obviously I am biased by the JDBC API which would like to have
PreparedStatement.execute() return the number of rows inserted
without having to wait to read all of the rows returned

Umm ... you do realize that we return the rows on-the-fly?

I do realize that.

The server does not know how many rows got inserted/returned

Well I haven't looked at the code, but it seems unintuitive that adding the
returning clause changes the semantics of insert.

until it's run the query to completion, at which point all

the data has already been sent to the client. There isn't
any way to return the rowcount before the data, and it wouldn't
be some trivial protocol adjustment to make that work differently.
(What it *would* be is expensive, because we'd have to store
those rows somewhere.)

I wasn't asking for that, I just want the number of rows inserted.

Thanks,

Dave

On Wed, Jul 12, 2023 at 2:59 PM Dave Cramer <davecramer@gmail.com> wrote:

On Wed, 12 Jul 2023 at 17:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dave Cramer <davecramer@gmail.com> writes:

Obviously I am biased by the JDBC API which would like to have
PreparedStatement.execute() return the number of rows inserted
without having to wait to read all of the rows returned

Umm ... you do realize that we return the rows on-the-fly?

I do realize that.

The server does not know how many rows got inserted/returned

Well I haven't looked at the code, but it seems unintuitive that adding
the returning clause changes the semantics of insert.

It doesn't have to - the insertions are always "as rows are produced", it
is just that in the non-returning case the final row can be sent to
/dev/null instead of the client (IOW, there is always some destination).
In both cases the total number of rows inserted are only reliably known
when the top executor node requests a new tuple and its immediate
predecessor says "no more rows present".

David J.

Dave Cramer <davecramer@gmail.com> writes:

Obviously I am biased by the JDBC API which would like to have
PreparedStatement.execute() return the number of rows inserted
without having to wait to read all of the rows returned

Huh ... just how *is* PreparedStatement.execute() supposed
to behave when the statement is an INSERT RETURNING?

execute() -> true
getResultSet() -> the rows
getMoreResults() -> false
getUpdateCount() -> number inserted?

It seems that would fit the portal's behavior easily enough.

Or is the JDBC spec insisting on some other order?

Regards,
-Chap

On Wed, 12 Jul 2023 at 20:00, <chap@anastigmatix.net> wrote:

Dave Cramer <davecramer@gmail.com> writes:

Obviously I am biased by the JDBC API which would like to have
PreparedStatement.execute() return the number of rows inserted
without having to wait to read all of the rows returned

Huh ... just how *is* PreparedStatement.execute() supposed
to behave when the statement is an INSERT RETURNING?

It's really executeUpdate which is supposed to return the number of rows
updated.
Without a cursor it returns right away as all of the results are returned
by the server. However with cursor you have to wait until you fetch the
rows before you can get the CommandComplete message which btw is wrong as
it returns INSERT 0 0 instead of INSERT 2 0

Dave

On Wed, Jul 12, 2023 at 5:57 PM Dave Cramer <davecramer@gmail.com> wrote:

On Wed, 12 Jul 2023 at 20:00, <chap@anastigmatix.net> wrote:

Dave Cramer <davecramer@gmail.com> writes:

Obviously I am biased by the JDBC API which would like to have
PreparedStatement.execute() return the number of rows inserted
without having to wait to read all of the rows returned

Huh ... just how *is* PreparedStatement.execute() supposed
to behave when the statement is an INSERT RETURNING?

It's really executeUpdate which is supposed to return the number of rows
updated.

Right, and executeUpdate is the wrong API method to use, in the PostgreSQL
world, when executing insert/update/delete with the non-SQL-standard
returning clause. executeQuery is the method to use. And execute() should
behave as if executeQuery was called, i.e., return true, which it is
capable of doing since it has resultSet data that it needs to handle.

The addition of returning turns the insert/update/delete into a select in
terms of effective client-seen behavior.

ISTM that you are trying to make user-error less painful. While that is
laudable it apparently isn't practical. They can either discard the
results and get a count by omitting returning or obtain the result and
derive the count by counting rows alongside whatever else they needed the
returned data for.

David J.

On Wed, 12 Jul 2023 at 21:31, David G. Johnston <david.g.johnston@gmail.com>
wrote:

On Wed, Jul 12, 2023 at 5:57 PM Dave Cramer <davecramer@gmail.com> wrote:

On Wed, 12 Jul 2023 at 20:00, <chap@anastigmatix.net> wrote:

Dave Cramer <davecramer@gmail.com> writes:

Obviously I am biased by the JDBC API which would like to have
PreparedStatement.execute() return the number of rows inserted
without having to wait to read all of the rows returned

Huh ... just how *is* PreparedStatement.execute() supposed
to behave when the statement is an INSERT RETURNING?

It's really executeUpdate which is supposed to return the number of rows
updated.

Right, and executeUpdate is the wrong API method to use, in the PostgreSQL
world, when executing insert/update/delete with the non-SQL-standard
returning clause. executeQuery is the method to use. And execute() should
behave as if executeQuery was called, i.e., return true, which it is
capable of doing since it has resultSet data that it needs to handle.

The addition of returning turns the insert/update/delete into a select in
terms of effective client-seen behavior.

ISTM that you are trying to make user-error less painful. While that is
laudable it apparently isn't practical. They can either discard the
results and get a count by omitting returning or obtain the result and
derive the count by counting rows alongside whatever else they needed the
returned data for.

Any comment on why the CommandComplete is incorrect ?
It returns INSERT 0 0 if a cursor is used

Dave

On Thursday, July 13, 2023, Dave Cramer <davecramer@gmail.com> wrote:

Any comment on why the CommandComplete is incorrect ?
It returns INSERT 0 0 if a cursor is used

Looking at DECLARE it is surprising that what you describe is even
possible. Can you share a psql reproducer?

David J.

On Thu, 13 Jul 2023 at 10:24, David G. Johnston <david.g.johnston@gmail.com>
wrote:

On Thursday, July 13, 2023, Dave Cramer <davecramer@gmail.com> wrote:

Any comment on why the CommandComplete is incorrect ?
It returns INSERT 0 0 if a cursor is used

Looking at DECLARE it is surprising that what you describe is even
possible. Can you share a psql reproducer?

apologies, we are using a portal, not a cursor.
Dave Cramer

On Thu, Jul 13, 2023 at 6:07 PM Dave Cramer <davecramer@gmail.com> wrote:

On Thu, 13 Jul 2023 at 10:24, David G. Johnston <
david.g.johnston@gmail.com> wrote:

On Thursday, July 13, 2023, Dave Cramer <davecramer@gmail.com> wrote:

Any comment on why the CommandComplete is incorrect ?
It returns INSERT 0 0 if a cursor is used

Looking at DECLARE it is surprising that what you describe is even
possible. Can you share a psql reproducer?

apologies, we are using a portal, not a cursor.

Still the same basic request of providing a reproducer - ideally in psql.

IIUC a portal has to be used for a prepared (extended query mode) result
set returning query. v16 can now handle parameter binding so:

postgres=# \bind 4
postgres=# insert into ins values ($1) returning id;
id
----
4
(1 row)

INSERT 0 1

Which gives the expected non-zero command tag row count result.

David J.

On 2023-07-12 20:57, Dave Cramer wrote:

Without a cursor it returns right away as all of the results are
returned
by the server. However with cursor you have to wait until you fetch the
rows before you can get the CommandComplete message which btw is wrong
as
it returns INSERT 0 0 instead of INSERT 2 0

To make sure I am following, was this describing a comparison of
two different ways in Java, using JDBC, to perform the same operation,
one of which behaves as desired while the other doesn't? If so, for
my curiosity, what do both ways look like in Java?

Or was it a comparison of two different operations, say one
an INSERT RETURNING and the other something else?

Regards,
-Chap

David,

I will try to get a tcpdump file. Doing this in libpq seems challenging as
I'm not aware of how to create a portal in psql.

Chap

The only difference is one instance uses a portal to fetch the results, the
other (correct one) is a normal insert where all of the rows are returned
immediately

this is a reproducer in Java

conn.prepareStatement("DROP TABLE IF EXISTS test_table").execute();
conn.prepareStatement("CREATE TABLE IF NOT EXISTS test_table (id
SERIAL PRIMARY KEY, cnt INT NOT NULL)").execute();

for (var fetchSize : List.of(0, 1, 2, 3)) {
System.out.println("FetchSize=" + fetchSize);

try (var stmt = conn.prepareStatement("INSERT INTO test_table
(cnt) VALUES (1), (2) RETURNING id", RETURN_GENERATED_KEYS)) {
stmt.setFetchSize(fetchSize);

var ret = stmt.executeUpdate();
System.out.println("executeUpdate result: " + ret);

var rs = stmt.getGeneratedKeys();
System.out.print("ids: ");
while (rs.next()) {
System.out.print(rs.getInt(1) + " ");
}
System.out.print("\n\n");
}
}

Dave

On Fri, 14 Jul 2023 at 12:07, <chap@anastigmatix.net> wrote:

On Fri, Jul 14, 2023 at 9:30 AM Dave Cramer <davecramer@gmail.com> wrote:

David,

I will try to get a tcpdump file. Doing this in libpq seems challenging as
I'm not aware of how to create a portal in psql.

Yeah, apparently psql does something special (like ignoring it...) with its
FETCH_COUNT variable (set to 2 below as evidenced by the first query) for
the insert returning case. As documented since the command itself is not
select or values the test in is_select_command returns false and the branch:

else if (pset.fetch_count <= 0 || pset.gexec_flag ||
pset.crosstab_flag || !is_select_command(query))
{
/* Default fetch-it-all-and-print mode */

Is chosen.

Fixing that test in some manner and recompiling psql seems like it should
be the easiest way to produce a core-only test case.

postgres=# select * from (Values (1),(2),(30000),(40000)) vals (v);
v
---
1
2
30000
40000
(4 rows)

postgres=# \bind 5 6 70000 80000
postgres=# insert into ins values ($1),($2),($3),($4) returning id;
id
-------
5
6
70000
80000
(4 rows)

INSERT 0 4

I was hoping to see the INSERT RETURNING query have a 4 width header
instead of 7.

David J.

On Fri, Jul 14, 2023 at 9:50 AM David G. Johnston <
david.g.johnston@gmail.com> wrote:

Fixing that test in some manner and recompiling psql seems like it should
be the easiest way to produce a core-only test case.

Apparently not - since it (ExecQueryUsingCursor) literally wraps the query
in a DECLARE CURSOR SQL Command which prohibits INSERT...

I suppose we'd have to write a psql equivalent of ExecQueryUsingPortal that
iterates over via fetch to make this work...probably more than I'm willing
to try.

David J.

See attached pcap file

after the execute of the portal it returns INSERT 0 0
Dave Cramer

On Fri, 14 Jul 2023 at 12:57, David G. Johnston <david.g.johnston@gmail.com>
wrote:

On 2023-07-14 12:58, Dave Cramer wrote:

See attached pcap file

So if the fetch count is zero and no portal is needed,
or if the fetch count exceeds the row count and the command
completion follows directly with no suspension of the portal, then
it comes with the correct count, but if the portal gets suspended,
then the later command completion reports a zero count?

Regards,
-Chap