search

cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.11k stars 3.81k forks source link

distsql: subqueries should be run through distsql as well #28263

Closed jordanlewis closed 6 years ago

jordanlewis commented 6 years ago

This is a follow-on to #27752.

Merging the two execution engines requires that we don't touch any given planNode execution path unless DistSQL doesn't support that particular planNode. The linked PR permits queries with subqueries in them to be distributed after the subqueries run, but the subqueries themselves are still run through local sql. This needs to be corrected.

rjnn commented 6 years ago

One problem with this is that expression evaluation happens during planning time, when none of the infrastructure for running distsql plans exists. One solution is to have the distsql planning process return the regular plan and a slice of closures. These closures can be supplied DistSQL receivers, which will run the subqueries and replace the expressions with the results. One downside here is that if there are many subqueries, we are returning a lot of closures.

Alternatively we can simply plumb all the required execution infrastructure through the planner, so that it is available in MakeExpression, where the subqueries are run.

Do you have a strong preference? both involve significant plumbing work, so it will be rather messy, so I'd like some reviewer buy in before sending out a PR.

jordanlewis commented 6 years ago

One problem with this is that expression evaluation happens during planning time

What do you mean? The new subqueries-in-distsql code seems to evaluate subqueries during DistSQLPlanner.PlanAndRun, which should have the smarts that it takes to plan new queries. Right? What happens if you call dsp.PlanAndRun recursively on all of the subquery plans in p.curPlan?

asubiotto commented 6 years ago

Closed by https://github.com/cockroachdb/cockroach/pull/28580