Proposal: Fail a statement test when an actual output is a query result

[melgenek]

https://github.com/risinglightdb/sqllogictest-rs/blob/main/sqllogictest/src/runner.rs#L627-L628

            // Tolerate the mismatched return type...
            (Record::Statement { .. }, RecordOutput::Query { error: None, .. }) => {}

The statement currently treats a query output as a successful result.

In case a test suite evolves and has some records that were defined as statement ok, those will effectively skip result validation if a client returns results.

This could happen, for example, if a test table had initially been empty, but a test suite was updated later. In this case, select queries that used to produce no results start produce results. But sqllogictest test wouldn't fail in this case.

I propose to fail test validation if a record produces query results, but was defined as a statement. This way tests that become invalid over time would be noticed.

[xxchan]

I also wanted to change that before, until I found there's actually an existing use case..

https://github.com/risingwavelabs/risingwave/blob/f6028e482b3c6928a605e75621bda565ae80f45d/e2e_test/batch/functions/internal.slt.part#L13-L14

statement ok
select rw_vnode(_row_id) as vnode, _row_id from t;

It seems they want to express the situation where they don't care about the results if the query doesn't fail. (And the results might be non-deterministic.) If the results matter, we can just use query instead.

I'm not sure whether the use case makes sense, and don't know other sqllogictest's behavior. But I'm open to such changes. 😄

Also cc @skyzh @wangrunji0408 for the reasons why we implemented such behavior.

[skyzh]

Actually we use this a lot in RisingLight, meaning that we don’t care about the result… For example,

statement ok select random();

I think we might also need to propose a syntax for “execute query without checking the result”. I’m not sure if this is consistent with the original SQLite test suite.

[skyzh]

I’m also thinking of calling the validator for statements and let the validator decide whether to fail.