sql: mutations in statement sources don't work properly with limits

[jordanlewis]

root@:26257/test> create table a (a int);
CREATE TABLE
root@:26257/test> insert into a values(1),(2);
INSERT 2
root@:26257/test> select * from [delete from a returning a] limit 1;
+---+
| a |
+---+
| 1 |
+---+
(1 row)
root@:26257/test> select * from [delete from a returning a] limit 2;
+---+
| a |
+---+
| 1 |
| 2 |
+---+
(2 rows)
root@:26257/test> select * from a;
+---+
| a |
+---+
| 1 |
| 2 |
+---+
(2 rows)

As you can see, the inner delete never gets executed.

Without a limit, or with an offset, this seems to work properly:

root@:26257/test> select * from [delete from a returning a] offset 1;
+---+
| a |
+---+
| 2 |
+---+
(1 row)
root@:26257/test> select * from a;
+---+
| a |
+---+
+---+
(0 rows)

Same problem exists with insert:

root@:26257/test> select * from [insert into a values(1) returning a] limit 1;
+---+
| a |
+---+
| 1 |
+---+
(1 row)
root@:26257/test> select * from a;
+---+
| a |
+---+
+---+
(0 rows)

cc @knz

[jordanlewis]

The problem is that we don't flush the result when the plan is closed. The result is only flushed currently when the DML's datasource is exhausted. I think this is an okay change to make. I'll send it out for review.

[jordanlewis]

So I tried fixing this but it wasn't as straightforward as I was hoping. cc @knz do you expect an inner [] plan to be fully consumed despite outer limits?

If so, and I think we should keep that expectation, we'll need to add a consume plan step when a plan is closed. I tried going down this path, but it's not straightforward - the right place for this seems like it should be in Close(), but Close() doesn't return an error and is perhaps not intended for these kinds of heavy-duty cleanups.

The resolution of this is a blocker for #20359, since common table expressions have the same problem - DML CTEs should be fully consumed in a plan.

[bdarnell]

Terminology nit: SELECT is a DML statement too. DML is defined in contrast to DDL (schema definitions); it doesn't have anything to do with read-only vs read/write statements.

[knz]

will followup in #23156