sql: add sort and virtual scan support in the new factory

[yuzefovich]

sql: implement ConstructSort in the new factory

Release note: None

sql: add support of virtual scans in the new factory

The support is added by sharing the same code with the old factory that
construct delayedNode with the rest of necessary methods already being
implemented in the new factory. That delayedNode is wrapped into the
physical plan via a callback.

Note that with the new factory it is possible to have a distributed plan
that contains an execinfra.LocalProcessor. This required to make
a change that distsql.LocalState, regardless of the plan distribution,
has always LocalProcs set when setting up a flow on the gateway.

Addresses: #47473.

Release note: None

[cockroach-teamcity]

This change is 

[asubiotto]

Reviewed 3 of 3 files at r1, 4 of 4 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @yuzefovich)

---

pkg/sql/distsql_spec_exec_factory.go, line 69 at r2 (raw file):

			evalCtx := e.planner.ExtendedEvalContext()
			e.planContexts.distPlanCtx = e.dsp.NewPlanningCtx(evalCtx.Context, evalCtx, e.planner.txn, distribute)
			e.planContexts.distPlanCtx.planner = e.planner

Why not have NewPlanningCtx set the planner field?

[yuzefovich]

Note that the new factory can now produce a distributed plan with a local plan node processor:

I think it is ok for now, but we might want to revisit this decision later - maybe we'll want to recreate such plans with forced local planning.

PTAL.

[yuzefovich]

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

---

pkg/sql/distsql_spec_exec_factory.go, line 69 at r2 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

Why not have NewPlanningCtx set the planner field?

Done.

[RaduBerinde]

Note that the new factory can now produce a distributed plan with a local plan node processor:

Nice, this seems like an improvement to me in general.

[yuzefovich]

Oh man, I can't figure out why the build is failing :/ I'll put off this PR for now.

[RaduBerinde]

Found a panic through the build's logTestLogic artifacts:

https://teamcity.cockroachdb.com/repository/download/Cockroach_UnitTests_Test/2075691:id/logTestLogic133547767/logictesttest-stderr.f407bf20ec37.roach.2020-07-08T23_09_09Z.014414.log

panic: flow: unexpected 10240 leftover bytes [recovered]
	panic: flow: unexpected 10240 leftover bytes

goroutine 9661329 [running]:
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).closeWrapper(0xc0851de000, 0x4bea240, 0xc02804abc0, 0x3f0d440, 0xc08e730f60)
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:799 +0x359
github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn.func1(0xc0851de000, 0x4bea240, 0xc02804abc0)
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:489 +0x61
panic(0x3f0d440, 0xc08e730f60)
	/usr/local/go/src/runtime/panic.go:969 +0x166
github.com/cockroachdb/cockroach/pkg/util/log.ReportOrPanic(0x4bea240, 0xc01d7744c0, 0xc03b9fca80, 0x42070a2, 0x20, 0xc0083c00e0, 0x2, 0x2)
	/go/src/github.com/cockroachdb/cockroach/pkg/util/log/crash_reporting.go:335 +0x23c
github.com/cockroachdb/cockroach/pkg/util/mon.(*BytesMonitor).doStop(0xc0422d1220, 0x4bea240, 0xc01d7744c0, 0x163f101)
	/go/src/github.com/cockroachdb/cockroach/pkg/util/mon/bytes_usage.go:397 +0x2c3
github.com/cockroachdb/cockroach/pkg/util/mon.(*BytesMonitor).Stop(...)
	/go/src/github.com/cockroachdb/cockroach/pkg/util/mon/bytes_usage.go:384
github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*EvalContext).Stop(...)
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/eval.go:3206
github.com/cockroachdb/cockroach/pkg/sql/flowinfra.(*FlowBase).Cleanup(0xc07790d8c0, 0x4bea240, 0xc01d7744c0)
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/flowinfra/flow.go:422 +0x78
github.com/cockroachdb/cockroach/pkg/sql/colflow.(*vectorizedFlow).Cleanup(0xc04f3e19e0, 0x4bea240, 0xc01d7744c0)
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/vectorized_flow.go:309 +0x1ea
github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).Run.func7()
	/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:426 +0xcf
github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine(0xc0851de000, 0x4bea300, 0xc03d0396b0, 0xc0851de3d8, 0x3, 0x7f1b40423c30, 0xc03fea2680, 0xc0083c0401, 0xc041a372b8, 0x0, ...)

[yuzefovich]

Oh, thanks. I'm having really hard time navigating the logs now. This panic seems related to a test logic failure on master #51179 (comment).

[yuzefovich]

Ok, the build is green (I needed to fix a thing with scrub checks, and another failure is unrelated problem present on master), so I'm going to go ahead and merge this.

TFTR!

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)