cli: handle rows and error

[RaduBerinde]

I have a usecase for a statement which can return some information as a result, as well as an error.

In the cli, depending on the display format, these rows may or may not show up:

• raw, record shows the rows
• the default tabular format never shows the rows
• csv, tsv didn't show the rows in practice (but @knz points me to cli: periodically flush csv/tsv output #28688 so it might show some rows in some cases).

We should make these all consistent and always show the rows even if we do receive an error.

I tested this with the code below for a specific statement in the sql executor:

		res.DisableBuffering()
		for i := 1; i < 10; i++ {
			res.AddRow(ctx, tree.Datums{tree.NewDString(fmt.Sprintf("some text %d", i))})
		}
		err := fmt.Errorf("FOO BAR")
		res.SetError(err)
		return makeErrEvent(err)

[knz]

Do you have a specific preference for returning results + error, as opposed to results + notice? (#45679)

I am asking because even though we could teach our CLI shell to report both partial results and error, this is not simple to do with every driver. If you plan to deliver a functionality usable cross-client, notices are the best way to go.

[RaduBerinde]

For some background, I'm working on a new EXPLAIN BUNDLE statement which runs the query (similarly to EXPLAIN ANALYZE) and then returns some information text with a URL to a zip bundle which contains a bunch of information. I think results + notice would actually work great here (it would allow us to still return the original results).

The question is what do we do in case of an error? Currently we just return the error. Ideally we'd still create the bundle and return the information about it. We don't want to eat the error because that would prevent auto-retries. Is it possible to return error + notice? Otherwise I might just wrap the error to add the detail about the bundle.

[RaduBerinde]

I tried it and the notice doesn't show up when there's an error. That behavior seems consistent with psql so we probably don't want to change it.

[knz]

The question is what do we do in case of an error?

Put the link to the plan as a DETAIL or HINT field in the error payload!

[RaduBerinde]

Yeah that's what I meant when I mentioned wrapping the error. I think that would work nicely.

[RaduBerinde]

There is a related issue here: where does a Notice show up (assuming it was added on the server-side after the results)? With the default formatter (as well as tsv, csv), it shows up before the results which is a little surprising (and not sure what happens with tsv/csv when the buffer gets flushed - does it show up in the middle?!). With raw, records, html it shows up at the end.

I'm worried that if the query returns many results, the notice will be missed if it is not at the end.

[knz]

We can enhance the SQL shell to repeat the notices at the end if there was more than a screenful of result rows.

[RaduBerinde]

Why not always show it at the end (at least in the case where the server sends the notice after the results)?

[knz]

Why not always show it at the end (at least in the case where the server sends the notice after the results)?

yeah in the case where the notice arrives after the first result row, I agree. But there could be a notice to inform a user that the first result is going to take a while to arrive, and that notice should be delivered immediately.

[knz]

I discovered that the code always delays notices until the end already.
So there is no more work here after #45872. closing.

[knz]

woops no I talked too soon.