Support multiple rows for DML RETURNING

[mrotteveel]

Currently, DML RETURNING only supports singleton inserts/updates/deletes. This restriction should be relaxed so that the following statements can return multiple rows:

• INSERT ... SELECT ...
• Searched UPDATE
• Searched DELETE
• MERGE
• UPDATE OR INSERT

The following can remain as-is (though maybe a unified implementation makes more sense?):

• INSERT ... DEFAULT VALUES / INSERT ... VALUES (...) (at least, until Firebird supports table value constructors)
• Positioned UPDATE (with WHERE CURRENT OF ...)
• Positioned DELETE (with WHERE CURRENT OF ...)

I suggest the implementation describes itself as isc_info_sql_stmt_select; this should make it compatible with most libraries and tools that rely on the statement type to decide on how to execute. The alternative is to introduce a new type (or types), for example isc_info_sql_stmt_dml_returning or something, but I think that makes things harder because tools and libraries would need to be updated to support this.

When such a statement is executed, Firebird should execute the statement to completion and collect all requested data in a type of temporary table, once execution is complete, fetches are done against this temporary table. In other words, even if no rows are fetched, all DML changes have been performed.

In PSQL, these statements should use the current singleton behaviour.

Non-singleton FOR ... RETURNING in PSQL is tracked in #6925.

[asfernandes]

Should we have some option (config / DPB / SET) to maintain DSQL compatibility?

[AlexPeshkoff]

On 5/17/21 4:39 PM, Adriano dos Santos Fernandes wrote: Should we have some option (config / DPB / SET) to maintain DSQL compatibility?

If there will be such need - certainly, if not - what sense in it?

[dyemanov]

I don't think it should be configurable.

[asfernandes]

I don't think it should be configurable.

I don't remember exact reason for why it was done as singleton.

Was it due to extra remote protocol roundtrip?

[aafemt]

Problem with partially fetched result set.

[dyemanov]

I don't think it should be configurable.

I don't remember exact reason for why it was done as singleton.

Was it due to extra remote protocol roundtrip?

Exactly. And RETURNING was primarily intended for a INSERT ... VALUES statement which is singular anyway.

[mrotteveel]

To address the roundtrip thing, we could consider introducing a new execute method which will stream all rows without requiring fetches, but that could be overkill.

[mrotteveel]

Problem with partially fetched result set.

I specifically address that in my proposal:

When such a statement is executed, Firebird should execute the statement to completion and collect all requested data in a type of temporary table, once execution is complete, fetches are done against this temporary table. In other words, even if no rows are fetched, all DML changes have been performed.

[asfernandes]

When such a statement is executed, Firebird should execute the statement to completion and collect all requested data in a type of temporary table, once execution is complete, fetches are done against this temporary table. In other words, even if no rows are fetched, all DML changes have been performed.

I want more opinions on this.

I understand the problem you want to avoid, specially in remote protocol which things can be buffered.

But it will make DSQL and PSQL with very different behavior.

Assuming we have a procedure P2 doing FOR UPDATE ... RETURNING ... SUSPEND and P1 calling it (with FOR SELECT) and interrupting before all rows are selected, it will do partial update and seems ok.

But with an EXECUTE BLOCK doing FOR UPDATE ... RETURNING ... SUSPEND and client interrupting it, it will still have undefined behavior on how many records were updated.

[hvlad]

FOR UPDATE\DELETE ... RETURNING could (should?) also buffer all returned rows before getting them out

[mrotteveel]

I think even in PSQL, a FOR UPDATE ... RETURNING ... should perform the entire update and cache/store the result set before emitting rows.

[asfernandes]

I have split the PSQL FOR in #6925, so we can start with the DSQL changes.

[pavel-zotov]

::: test details :::

Test verifies only basic issues related mostly to syntax and ability to get multiple rows.
Both DSQL and PSQL are tested.

Interruption of fetching process by client (and check number of affected rows) is NOT verified:
separate test will be made later (see ticket, comment by Adriano, date: 15-JUL-2021).

[asfernandes]

::: test details :::

Test verifies only basic issues related mostly to syntax and ability to get multiple rows.
Both DSQL and PSQL are tested.

Interruption of fetching process by client (and check number of affected rows) is NOT verified:
separate test will be made later (see ticket, comment by Adriano, date: 15-JUL-2021).

@pavel-zotov to make things clear, the atomic changes, then returning, is already implemented for the DSQL case.

And for PSQL without FOR (tracked in #6925), things should work as before.