distsql: the Context used for expression evaluation (EvalCtx.Ctx) is wrong

[andreimatei]

As indicated by the TODO here: https://github.com/cockroachdb/cockroach/blob/549631b/pkg/sql/distsqlrun/server.go#L180

Each processor that needs to evaluate expressions should initialize the Ctx inside the EvalCtx. This would probably be made easier by making the EvalCtx immutable and passing it by value more.

[cuongdo]

This still seems important and should be done earlier, to avoid another last-minute scramble. Is this doable for 1.1?

[andreimatei]

I don't know how much work it is to fix this, I have to look at the code. But I don't think it's a particularly big deal; I think this just affects the log messages from expression evaluation.

[RaduBerinde]

Ah, from the description it seemed that this is about an EvalContext being wrong, but it's about a context.Context, so it's not really that important. Moving to 1.2.

[jordanlewis]

Seems out of date - either fixed or different now. Closing.

[andreimatei]

Why does it seem out of date or different? If you look for uses of evalCtx.Ctx() in distsql you find plenty - e.g.

cockroach/pkg/sql/distsqlrun/sorter.go

Line 225 in b1d18df

ctx := s.evalCtx.Ctx()

Also all the expression evaluation done by DistSQL.

[andreimatei]

Looks like this was fixed for most processors at some point:
https://github.com/cockroachdb/cockroach/blob/e207532786239891f1581da559c8c401c6c6b408/pkg/sql/execinfra/processorsbase.go#L805