sql: help the user understand unsupported correlation

Prior to this patch, a client trying to use an unsupported correlated
query would encounter an obscure error like "column v does not exist"
or "no data source matches prefix".

Given that the new optimizer code can determine whether a query is
correlated, we can use this information to enhance the error message.

Before:

```
> select * from pg_class a where exists (select * from pg_class b where a.oid = b.oid);
pq: no data source matches prefix: a

> select * from pg_class a where exists (select * from kv where v::oid = oid);
pq: column "oid" does not exist
```

After:

```
> select * from pg_class a where exists (select * from pg_class b where a.oid = b.oid);
pq: no data source matches prefix: a
HINT: some correlated subqueries are not supported yet - see
      cockroachdb#3288

> select * from pg_class a where exists (select * from kv where v::oid = oid);
pq: column "oid" does not exist
HINT: some correlated subqueries are not supported yet - see
      cockroachdb#3288
```

Note: some correlated queries do not benefit from this improvement,
specifically those for which the optimizer code aborts early before it
has detected correlation (e.g. because of some other unrelated
feature).

Release note (sql change): CockroachDB will now report a hint in the
error message if it encounters a correlated query that it does not
support yet.

pkg/sql/conn_executor_exec.go

@@ -680,6 +680,37 @@ func (ex *connExecutor) execStmtInParallel(
 	return cols, nil
 }
 
+func enhanceErrWithCorrelation(err error, isCorrelated bool) {
+	if err == nil || !isCorrelated {
+		return
+	}
+
+	// If the query was found to be correlated by the new-gen
+	// optimizer, but the optimizer decided to give up (e.g. because
+	// of some feature it does not support), in most cases the
+	// heuristic planner will choke on the correlation with an
+	// unhelpful "table/column not defined" error.
+	//
+	// ("In most cases" because the heuristic planner does support
+	// *some* correlation, specifically that of SRFs in projections.)
+	//
+	// To help the user understand what is going on, we enhance these
+	// error message here when correlation has been found.
+	//
+	// We cannot be more assertive/definite in the text of the hint
+	// (e.g. by replacing the error entirely by "correlated queries are
+	// not supported") because perhaps there was an actual mistake in
+	// the query in addition to the unsupported correlation, and we also
+	// want to give a chance to the user to fix mistakes.
+	if pqErr, ok := err.(*pgerror.Error); ok {
+		if pqErr.Code == pgerror.CodeUndefinedColumnError ||
+			pqErr.Code == pgerror.CodeUndefinedTableError {
+			pqErr.SetHintf("some correlated subqueries are not supported yet - see %s",
+				"https://github.com/cockroachdb/cockroach/issues/3288")
+		}
+	}
+}
+
 // dispatchToExecutionEngine executes the statement, writes the result to res
 // and returns an event for the connection's state machine.
 //
@@ -696,8 +727,11 @@ func (ex *connExecutor) dispatchToExecutionEngine(
 
 	optimizerPlanned, err := planner.optionallyUseOptimizer(ctx, ex.sessionData, stmt)
 	if !optimizerPlanned && err == nil {
+		isCorrelated := planner.curPlan.isCorrelated
+		log.VEventf(ctx, 1, "query is correlated: %v", isCorrelated)
 		// Fallback if the optimizer was not enabled or used.
 		err = planner.makePlan(ctx, stmt)
+		enhanceErrWithCorrelation(err, isCorrelated)
 	}
 
 	defer func() { planner.maybeLogStatement(ctx, "exec", res.RowsAffected(), res.Err()) }()

pkg/sql/conn_executor_prepare.go

@@ -212,8 +212,11 @@ func (ex *connExecutor) prepare(
 		if optimizerPlanned, err := p.optionallyUseOptimizer(ctx, ex.sessionData, stmt); err != nil {
 			return err
 		} else if !optimizerPlanned {
+			isCorrelated := p.curPlan.isCorrelated
+			log.VEventf(ctx, 1, "query is correlated: %v", isCorrelated)
 			// Fallback if the optimizer was not enabled or used.
 			if err := p.prepare(ctx, stmt.AST); err != nil {
+				enhanceErrWithCorrelation(err, isCorrelated)
 				return err
 			}
 		}

pkg/sql/logictest/testdata/logic_test/subquery_correlated

@@ -683,3 +683,9 @@ SELECT c.c_id, o.o_id
 FROM c
 INNER JOIN o
 ON c.c_id=o.c_id AND o.ship = (SELECT min(o.ship) FROM o WHERE o.c_id=c.c_id);
+
+statement error HINT: some correlated subqueries are not supported yet
+SELECT * FROM pg_class a WHERE EXISTS (SELECT * FROM pg_class b WHERE a.oid = b.oid)
+
+statement error HINT: some correlated subqueries are not supported yet
+SELECT * FROM pg_class a WHERE EXISTS (SELECT * FROM o WHERE a.oid = o.o_id::oid)

pkg/sql/opt/optbuilder/builder.go

@@ -76,6 +76,15 @@ type Builder struct {
 	// case.
 	FmtFlags tree.FmtFlags
 
+	// IsCorrelated is set to true during semantic analysis if a scalar variable was
+	// pulled from an outer scope, that is, if the query was found to be correlated.
+	IsCorrelated bool
+
+	// UsingVirtualTable is set to true during semantic analysis if a
+	// FROM clause refers to a virtual table.
+	// TODO(andyk/knz): Remove when the builder supports virtual tables.
+	UsingVirtualTable bool
+
 	factory *norm.Factory
 	stmt    tree.Statement
 

pkg/sql/opt/optbuilder/project.go

@@ -185,6 +185,10 @@ func (b *Builder) finishBuildScalar(
 func (b *Builder) finishBuildScalarRef(
 	col *scopeColumn, label string, inScope, outScope *scope,
 ) (out memo.GroupID) {
+	isOuterColumn := inScope.isOuterColumn(col.id)
+	// Remember whether the query was correlated for later.
+	b.IsCorrelated = b.IsCorrelated || isOuterColumn
+
 	// If this is not a projection context, then wrap the column reference with
 	// a Variable expression that can be embedded in outer expression(s).
 	if outScope == nil {
@@ -193,7 +197,7 @@ func (b *Builder) finishBuildScalarRef(
 
 	// Outer columns must be wrapped in a variable expression and assigned a new
 	// column id before projection.
-	if inScope.isOuterColumn(col.id) {
+	if isOuterColumn {
 		// Avoid synthesizing a new column if possible.
 		existing := outScope.findExistingCol(col)
 		if existing == nil {

pkg/sql/opt/optbuilder/select.go

@@ -72,9 +72,11 @@ func (b *Builder) buildTable(texpr tree.TableExpr, inScope *scope) (outScope *sc
 			panic(builderError{err})
 		}
 
-		// TODO(andyk): Re-enable virtual tables when we can fully support them.
+		// TODO(andyk,knz): Remove this determination once virtual tables
+		// are fully supported by the optimizer.
+		// See corresponding code in sql.makeOptimizerPlan().
 		if tab.IsVirtualTable() {
-			panic(unimplementedf("virtual tables are not supported"))
+			b.UsingVirtualTable = true
 		}
 
 		return b.buildScan(tab, tn, inScope)

pkg/sql/plan.go

@@ -271,6 +271,10 @@ type planTop struct {
 	// #10028 is addressed.
 	hasStar bool
 
+	// isCorrelated collects whether the query was found to be correlated.
+	// Used to produce better error messages.
+	isCorrelated bool
+
 	// subqueryPlans contains all the sub-query plans.
 	subqueryPlans []subquery
 
@@ -353,8 +357,8 @@ func (p *planner) makePlan(ctx context.Context, stmt Statement) error {
 // makeOptimizerPlan is an alternative to makePlan which uses the (experimental)
 // optimizer.
 func (p *planner) makeOptimizerPlan(ctx context.Context, stmt Statement) error {
-	// This will get overwritten if we successfully create a plan, but if we
-	// error we need access to the AST.
+	// Ensure that p.curPlan is populated in case an error occurs early,
+	// so that maybeLogStatement in the error case does not find an empty AST.
 	p.curPlan = planTop{AST: stmt.AST}
 
 	// Start with fast check to see if top-level statement is supported.
@@ -372,10 +376,22 @@ func (p *planner) makeOptimizerPlan(ctx context.Context, stmt Statement) error {
 	o := xform.NewOptimizer(p.EvalContext())
 	bld := optbuilder.New(ctx, &p.semaCtx, p.EvalContext(), &catalog, o.Factory(), stmt.AST)
 	root, props, err := bld.Build()
+
+	// Remember whether the plan was correlated before processing the
+	// error. This way, the executor will abort early with a useful error message instead of trying
+	// the heuristic planner.
+	p.curPlan.isCorrelated = bld.IsCorrelated
+
 	if err != nil {
 		return err
 	}
 
+	// TODO(andyk): Re-enable virtual tables when we can fully support them.
+	if bld.UsingVirtualTable {
+		return pgerror.Unimplemented("virtual table",
+			"virtual tables are not supported yet by the optimizer")
+	}
+
 	ev := o.Optimize(root, props)
 
 	factory := makeExecFactory(p)
@@ -385,7 +401,10 @@ func (p *planner) makeOptimizerPlan(ctx context.Context, stmt Statement) error {
 	}
 
 	p.curPlan = *plan.(*planTop)
+	// Since the assignment above just cleared the AST and isCorrelated
+	// field, we need to set them again.
 	p.curPlan.AST = stmt.AST
+	p.curPlan.isCorrelated = bld.IsCorrelated
 
 	cols := planColumns(p.curPlan.plan)
 	if stmt.ExpectedTypes != nil {