sqlutil: remove Query and QueryEx from InternalExecutor interface

[yuzefovich]

All usages of sqlutil.InternalExecutor.Query and
sqlutil.InternalExecutor.QueryEx have been refactored: most of
the changes are taking advantage of the iterator API, in a couple
of places we now use Exec and ExecEx methods since in those
places we only need the number of affected rows.

In a few places where we still need to buffer all rows, the caller
currently performs the buffering. In the follow-up commit
QueryBuffered and QueryBufferedEx will be added into the interface
(essentially renaming Query and QueryEx to make it explicit that the
buffering occurs), and the custom logic will be removed.

Addresses: #48595.

Release note: None

[cockroach-teamcity]

This change is 

[yuzefovich]

@andreimatei or @ajwerner, could one of you please take a look at the first commit?

@miretskiy, could you please take a look at the second commit?

[ajwerner]

neat

Reviewed 16 of 16 files at r1, 8 of 8 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @miretskiy)

---

pkg/jobs/adopt.go, line 283 at r1 (raw file):
Just rambles for myself:

Quoted 6 lines of code…

		// Note that we have to buffer all rows first - before processing each
		// job - because we have to make sure that the query executes without an
		// error (otherwise, the system.jobs table might diverge from the jobs
		// registry).
		// TODO(yuzefovich): use QueryBufferedEx method once it is added to
		// sqlutil.InternalExecutor interface.

I'm not so sure about this. We could just make it two loops but I guess at that point, it's the same complexity in terms of memory.

Also, we could add a LIMIT here if we wanted without violating correctness.

[yuzefovich]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @miretskiy)

---

pkg/jobs/adopt.go, line 283 at r1 (raw file):

Previously, ajwerner wrote…

Just rambles for myself:

		// Note that we have to buffer all rows first - before processing each
		// job - because we have to make sure that the query executes without an
		// error (otherwise, the system.jobs table might diverge from the jobs
		// registry).
		// TODO(yuzefovich): use QueryBufferedEx method once it is added to
		// sqlutil.InternalExecutor interface.

I'm not so sure about this. We could just make it two loops but I guess at that point, it's the same complexity in terms of memory.

Also, we could add a LIMIT here if we wanted without violating correctness.

I tried avoiding the buffering of rows here, but the CI was failing (I think something about auto stats collection, but don't remember for sure) which I bisected to this spot. The comment is my best explanation for why the buffering is needed.

[yuzefovich]

I removed the second commit and will merge it in a separate PR.

TFTR!

bors r+

[craig]

This PR was included in a batch that was canceled, it will be automatically retried

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build succeeded:

• GitHub CI (Cockroach)