I'm very happy to see this happening, great work!

Reviewed 72 of 74 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RaduBerinde and @stevendanna)

-- commits, line 35 at r2:
nit: it might be worth to call out this change with columns by name in a release note.

pkg/sql/colfetcher/colbatch_scan.go, line 210 at r2 (raw file):

	// Make sure that render expressions are deserialized right away so that we
	// don't have to re-parse them multiple times.
	if post.RenderExprs != nil {

Did you add this because it was in populateTableArgsLegacy? I think we should not include this here - over there, we need the render expressions to be deserialized because we might remap the indexed vars, but IIUC this PR doesn't have to do that.

pkg/sql/execinfra/version.go, line 42 at r2 (raw file):

// ATTENTION: When updating these fields, add a brief description of what
// changed to the version history below.
const Version execinfrapb.DistSQLVersion = 55

These also need to be bumped (we forgot to do it for version 56).

pkg/sql/execinfrapb/flow_diagram.go, line 155 at r2 (raw file):

		col, err := tbl.FindColumnWithID(colID)
		if i > 0 {
			b.WriteByte(',')

If we need to scan many columns, then the box will become pretty wide. Thoughts on writing one column per line? Or maybe have a limit of, say of up to 100 characters per line?

pkg/sql/row/fetcher.go, line 294 at r2 (raw file):

	var err error

	// Scan through the entire columns map to see which columns are

nit: the comment needs an update.

pkg/sql/row/fetcher.go, line 1112 at r2 (raw file):

// NextRowInto calls NextRow and copies the results into the given EncDatumRow
// slice according to the given column map.
// mapping.

nit: leftover.

pkg/sql/row/fetcher.go, line 1167 at r2 (raw file):

// mapping.
//
// Values for columns that are not in the map are ignored. cDatums in

nit: s/cDatums/Datums/.

pkg/sql/row/fetcher.go, line 1193 at r2 (raw file):

			continue
		}
		if err := encDatum.EnsureDecoded(col.GetType(), rf.alloc); err != nil {

Does col.GetType() return the correct type for the inverted column? It might be worth making sure that inverted_.* logic tests run through the row-by-row engine correctly (maybe they already do).

pkg/sql/opt/exec/execbuilder/testdata/aggregate, line 730 at r2 (raw file):

 ORDER BY message LIKE 'fetched:%' DESC, ordinality ASC
----
fetched: /xyz/zyx/3.0/2/1 -> <undecoded>

This seems suspicious - we're scanning INDEX zyx (z, y, x), and only need the value of x, so I think the previous output was correct.

pkg/sql/opt/exec/execbuilder/testdata/explain_analyze_plans, line 338 at r2 (raw file):

distribution: <hidden>
vectorized: <hidden>
rows read from KV: 1 (8 B)

These changes to stats also seem suspicious.

pkg/sql/opt/exec/execbuilder/testdata/schema_change_in_txn_nonmetamorphic, line 34 at r2 (raw file):

CPut /Table/56/1/101/1/1 -> /TUPLE/4:4:Decimal/0.00/2:6:Decimal/1101.2

query T

nit: this seems like an unrelated addition of a test.

pkg/sql/opt/exec/execbuilder/testdata/vectorize_local, line 72 at r2 (raw file):

distribution: <hidden>
vectorized: <hidden>
rows read from KV: 2 (16 B)

ditto

TFTR!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @stevendanna and @yuzefovich)

pkg/sql/colfetcher/colbatch_scan.go, line 210 at r2 (raw file):

Yup, removed.

pkg/sql/execinfra/version.go, line 42 at r2 (raw file):

🤦

pkg/sql/execinfrapb/flow_diagram.go, line 155 at r2 (raw file):

Done.

pkg/sql/row/fetcher.go, line 1193 at r2 (raw file):

Yeah, we're passing in the full column descriptors and the callers do the inverted column type replacement before setting up the fetcher.

pkg/sql/opt/exec/execbuilder/testdata/aggregate, line 730 at r2 (raw file):

In the optimizer, if a scan is constrained it must return the columns it is constrained on. This is why the plan above has the scan returning x,y,z and then it projects away column x. We could fix that in the optimizer if it's a problem - filed #75153.

pkg/sql/opt/exec/execbuilder/testdata/explain_analyze_plans, line 338 at r2 (raw file):

Great catch, I didn't notice that! I did not override the new method in the rowFetcherStatCollector. I now unembedded the row.Fetcher so that we have to implement all methods explicitly.

pkg/sql/opt/exec/execbuilder/testdata/vectorize_local, line 72 at r2 (raw file):

Fixed.

Reviewed 2 of 74 files at r2, 10 of 10 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde and @stevendanna)

pkg/sql/opt/exec/execbuilder/testdata/aggregate, line 730 at r2 (raw file):

I see, thanks.

pkg/sql/execinfrapb/flow_diagram_external_test.go, line 1 at r3 (raw file):

// Copyright 2018 The Cockroach Authors.

nit: s/2018/2022/.

pkg/sql/execinfrapb/flow_diagram_external_test.go, line 48 at r3 (raw file):

		  this_is_a_very_long_name_15 INT,
		  this_is_a_very_long_name_16 INT,
			x INT,

super nit: the indentation is different.

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @stevendanna and @yuzefovich)

pkg/sql/execinfrapb/flow_diagram_external_test.go, line 48 at r3 (raw file):

Done.