Good.

In term of protocol to driver like in https://mariadb.com/kb/en/mariadb/ok_packet/ what would change :
have a new "status flags" indicator that tell that there some Insert ids row

Then have insert id's rows like

• int<lenenc> insert id's
• int<lenenc> sequence length

Then a EOF packet to indicate end of rows ?

why can't this just a a normal result set, and OK packet indicate that "more results exist", followed by a result set with 3 fields (first insert id, autoincrement_increment,sequence-length)

It will be length of additional date before the data: "preceded by length of the information chunk" also in the same codding. So you just need to read till the end.

It is not a normal result set to keep things simple. Normal data set require metadata, and so on.

The possibly huge "additional data" can overflow a max_allowed_packet, I think encoding this thing as result set is safer. And yes, metadata, 3 bigints, is that a problem?

Also, the big plus of resultset is not extending the protocol even further. I think a good judgement needs to be applied before adding stuff there using arbitrary encoding that just seemed a good idea. At some point, we'll loose overview in these "good" ideas.

i tend to agree having a real resultset will add some 50bytes without breaking protocol, but other solution is fine too.

So to resumate changes in protocol :
COM_STMT_EXECUTE flag will have a new value :

0 no cursor
1 read only
2 cursor for update
4 scrollable cursor
*128 need insert ids*

response from server when "need insert ids" is set will be an OK_Packet with MORE_RESULTS_EXISTS set, followed by a Resultset with 2 columns (First-Insert-ID and Sequence-length)

revision-id: 23959479689f47bdfe5aa33cb6cd5e1171b5f8a8 (mariadb-10.2.2-128-g23959479689)
parent(s): 9ea5de30963dd16cec7190b8b2f77858f4a82545
committer: Oleksandr Byelkin
timestamp: 2017-01-17 15:47:17 +0100
message:

MDEV-11419: Report all INSERT ID for bulk operation INSERT

Send all Insert IDs of the buld operation to client (JDBC need it)

—

Review answered.

revision-id: 714ca49a8b29cb23cb6520574b0d09dd683e5ef6 (mariadb-10.2.4-66-g714ca49)
parent(s): aad15eae89e9700d4c1ed4c83a68f8c7b6775a27
committer: Oleksandr Byelkin
timestamp: 2017-03-16 09:10:28 +0100
message:

MDEV-11419: Report all INSERT ID for bulk operation INSERT

Send all Insert IDs of the buld operation to client (JDBC need it)

—

bb-10.2-MDEV-11419 on github

Left Review Comments on https://github.com/mariadb/server/commit/714ca49a8b29cb23cb6520574b0d09dd683e5ef6

Is it still needed, if we'll have INSERT ... RETURNING?

Depends on what INSERT ... RETURNING can do. Can it return all autoincrement IDs in that were inserted in a batch in an easy way, that does not require knowledge of the autoincrement field name, e.g via LAST_INSERT_ID()?

no, LAST_INSERT_ID() won't help, it returns the first generated number, from a previous insert. But _rowid will work:

INSERT ... RETURNING _rowid

this will do

I think that INSERT RETURNING is a much better solution for a couple of reasons:

• It can return the whole PK even if it's not a single AUTO_INCREMENT field
• It can return non-key values (useful if you insert the result of an SQL expression, or default values)

But it is not clear to me how it will handle the special case of values that are modified in a BEFORE INSERT trigger.
My preference would be to see the value after trigger execution (not a strong opinion).
Db2 has a special syntax for these cases, so you are able to retrieve the value before or after triggers execution.

f_razzoli, it might not be clear from the task description, but the only reason/requester for this functionality was JDBC, specifically Statement.RETURN_GENERATED_KEYS in a batch update. JDBC does not mandate to support getGeneratedKeys() after a batch, but we have supported it for a while

If the SQL were INSERT IGNORE (or other query where the insert may fail), will the reported IDs include the previous IDs? It appears not. This makes the feature only partially usable.

For example, when 'normalizing' a batch of strings, it would be quite handy to get back all the ids correctly, not unused ids for the already-normalized strings.

Not needed anymore, INSERT ... RETURNING can be used instead.

Thanks!
Alas, the one place I wanted to use it is apparently not allowed:
INSERT ... SELECT ...
RETURNING ...;
Or maybe it would be
INSERT ... RETURNING ...
SELECT ...;

It is, https://github.com/MariaDB/server/blob/10.5/mysql-test/main/insert_returning.test#L136

what exactly isn't working for you?

I was looking at the syntax – It seems to be disallowed. Perhaps it is just a documentation omission?
(I have not yet tried it.)

Would the RETURNING come before the SELECT? Or After?

Looking at the syntax in the KB? https://mariadb.com/kb/en/insertreturning/ shows that it's supported.

See the manual for the documented supported syntax and the test file (in my previous comment) for actual working examples.