sql: fix an internal error in EXPLAIN ANALYZE in some cases

[yuzefovich]

f2be108 made it so that we embed the plan gist factory into optPlanningCtx that we reuse across queries, which is achieved by initializing it before the execbuild and then resetting right after it. This can be problematic in some EXPLAIN scenarios where we call the captured GetExplainPlan function for cascades and triggers with createIfMissing=true option. That option means that we might attempt to run the execbuilding for a cascade or a trigger at a different point in time than the execbuilding of the main query. In particular, this can be the case for EXPLAIN ANALYZE if the cascade or trigger was short-circuited. (There is a separate issue to highlight these short-circuited cascades and triggers.) Regular EXPLAIN is unaffected because we create all needed plans eagerly in startExec method; also, nested cascades and triggers are unaffected because they didn't capture the gist factory.

This commit fixes the problem by removing the only reference to the reset gist factory from the explain factory.

Fixes: #138974.

Release note: None

[cockroach-teamcity]

This change is 

[mgartner]

pkg/sql/plan_opt.go line 891 at r1 (raw file):

		opc.gf.Init(f)
		defer func(f exec.Factory) {
			if explainFactory != nil {

I'm worried this closure or explainFactory will be heap allocated. I'll run some tests to try to figure that out.

[mgartner]

-- commits line 13 at r1:
What does it mean for a cascade or trigger to be short-circuited?

[yuzefovich]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mgartner)

---

-- commits line 13 at r1:

Previously, mgartner (Marcus Gartner) wrote…

What does it mean for a cascade or trigger to be short-circuited?

If we have a DELETE from parent that cascades into child, but the DELETE doesn't actually remove any rows from parent, then we "short-circuit" execution of the child cascade plan since the cascade, in effect, wasn't triggered. Right now under EXPLAIN ANALYZE we would call GetExplainPlan that "augments" the PlanFn of the main execFactory, which might have the gist factory around it.

If the cascade is not short-circuited, then we create plan via the "vanilla" PlanFn of the main execFactory during the query execution, so later on when we populate the EXPLAIN output GetExplainPlan returns already created plan, so we don't hit the gist factory that has been reset.

One possible alternative is to avoid the plan creation with short-circuited cascade. I'm currently working on that and it would also resolve #125509.

[yuzefovich]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mgartner)

---

-- commits line 13 at r1:

Previously, yuzefovich (Yahor Yuzefovich) wrote…

If we have a DELETE from parent that cascades into child, but the DELETE doesn't actually remove any rows from parent, then we "short-circuit" execution of the child cascade plan since the cascade, in effect, wasn't triggered. Right now under EXPLAIN ANALYZE we would call GetExplainPlan that "augments" the PlanFn of the main execFactory, which might have the gist factory around it.

If the cascade is not short-circuited, then we create plan via the "vanilla" PlanFn of the main execFactory during the query execution, so later on when we populate the EXPLAIN output GetExplainPlan returns already created plan, so we don't hit the gist factory that has been reset.

One possible alternative is to avoid the plan creation with short-circuited cascade. I'm currently working on that and it would also resolve #125509.

That alternative approach seems promising #139078, so let's hold off on this PR for now.