opt: clean up isCorrelated flag

In the planning code we are plumbing through an `isCorrelated` flag
used to enhance the error message when we fall back to the heuristic
planner and the query was correlated. Since we no longer fall back,
this flag is not needed anymore.

Release note: None

pkg/sql/conn_executor_exec.go

@@ -597,35 +597,6 @@ func (ex *connExecutor) rollbackSQLTransaction(ctx context.Context) (fsm.Event,
 	return eventTxnFinish{}, eventTxnFinishPayload{commit: false}
 }
 
-func enhanceErrWithCorrelation(err error, isCorrelated bool) error {
-	if err == nil || !isCorrelated {
-		return err
-	}
-
-	// 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 code := pgerror.GetPGCode(err); code == pgcode.UndefinedColumn || code == pgcode.UndefinedTable {
-		err = errors.WithHintf(err, "some correlated subqueries are not supported yet - see %s",
-			"https://github.com/cockroachdb/cockroach/issues/3288")
-	}
-	return err
-}
-
 // dispatchToExecutionEngine executes the statement, writes the result to res
 // and returns an event for the connection's state machine.
 //
@@ -756,35 +727,24 @@ func (ex *connExecutor) makeExecPlan(ctx context.Context, planner *planner) erro
 	// in error cases.
 	planner.curPlan = planTop{AST: stmt.AST}
 
-	var isCorrelated bool
 	if optMode := ex.sessionData.OptimizerMode; optMode != sessiondata.OptimizerOff {
 		log.VEvent(ctx, 2, "generating optimizer plan")
 		var result *planTop
 		var err error
-		result, isCorrelated, err = planner.makeOptimizerPlan(ctx)
-		if err == nil {
-			planner.curPlan = *result
-			return nil
-		}
-		if isCorrelated {
-			// Note: we are setting isCorrelated here because
-			// makeOptimizerPlan() can determine isCorrelated but fail with
-			// a non-nil error and a nil result -- for example, when it runs
-			// into an unsupported SQL feature that the HP supports, after
-			// having processed a correlated subquery (which the heuristic
-			// planner won't support).
-			planner.curPlan.flags.Set(planFlagOptIsCorrelated)
+		result, err = planner.makeOptimizerPlan(ctx)
+		if err != nil {
+			log.VEventf(ctx, 1, "optimizer plan failed: %v", err)
+			return err
 		}
-		log.VEventf(ctx, 1, "optimizer plan failed (isCorrelated=%t): %v", isCorrelated, err)
-		return err
+		planner.curPlan = *result
+		return nil
 	}
 
 	log.VEvent(ctx, 2, "optimizer disabled")
 	// Use the heuristic planner.
 	optFlags := planner.curPlan.flags
 	err := planner.makePlan(ctx)
 	planner.curPlan.flags |= optFlags
-	err = enhanceErrWithCorrelation(err, isCorrelated)
 	return err
 }
 

pkg/sql/conn_executor_prepare.go

@@ -204,11 +204,10 @@ func (ex *connExecutor) populatePrepared(
 	// fallback for all others, so this should be safe for the foreseeable
 	// future.
 	var flags planFlags
-	var isCorrelated bool
 	if optMode := ex.sessionData.OptimizerMode; optMode != sessiondata.OptimizerOff {
 		log.VEvent(ctx, 2, "preparing using optimizer")
 		var err error
-		flags, isCorrelated, err = p.prepareUsingOptimizer(ctx)
+		flags, err = p.prepareUsingOptimizer(ctx)
 		if err == nil {
 			log.VEvent(ctx, 2, "optimizer prepare succeeded")
 			// stmt.Prepared fields have been populated.
@@ -223,7 +222,6 @@ func (ex *connExecutor) populatePrepared(
 	// a plan for the statement to figure out the typing, then close the plan.
 	prepared.AnonymizedStr = anonymizeStmt(stmt.AST)
 	if err := p.prepare(ctx, stmt.AST); err != nil {
-		err = enhanceErrWithCorrelation(err, isCorrelated)
 		return 0, err
 	}
 

pkg/sql/opt/optbuilder/builder.go

@@ -74,10 +74,6 @@ type Builder struct {
 	// These fields are set during the building process and can be used after
 	// Build is called.
 
-	// 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
-
 	// HadPlaceholders is set to true if we replaced any placeholders with their
 	// values.
 	HadPlaceholders bool
@@ -130,6 +126,10 @@ type Builder struct {
 	// TODO(radu): modifying the AST in-place is hacky; we will need to switch to
 	// using AST annotations.
 	qualifyDataSourceNamesInAST bool
+
+	// isCorrelated is set to true if we already reported to telemetry that the
+	// query contains a correlated subquery.
+	isCorrelated bool
 }
 
 // New creates a new Builder structure initialized with the given

pkg/sql/opt/optbuilder/scalar.go

@@ -511,15 +511,11 @@ func (b *Builder) checkSubqueryOuterCols(
 		return
 	}
 
-	if !b.IsCorrelated {
-		// Remember whether the query was correlated for the heuristic planner,
-		// to enhance error messages.
-		// TODO(knz): this can go away when the HP disappears.
-		b.IsCorrelated = true
-
-		// Register the use of correlation to telemetry.
-		// Note: we don't blindly increment the counter every time this
-		// method is called, to avoid double counting the same query.
+	// Register the use of correlation to telemetry.
+	// Note: we don't blindly increment the counter every time this
+	// method is called, to avoid double counting the same query.
+	if !b.isCorrelated {
+		b.isCorrelated = true
 		telemetry.Inc(sqltelemetry.CorrelatedSubqueryUseCounter)
 	}
 

pkg/sql/plan.go

@@ -821,10 +821,6 @@ const (
 	// planFlagOptUsed is set if the optimizer was used to create the plan.
 	planFlagOptUsed planFlags = (1 << iota)
 
-	// planFlagIsCorrelated is set if the plan contained a correlated subquery.
-	// This is used to enhance the error fallback and for telemetry.
-	planFlagOptIsCorrelated
-
 	// planFlagOptCacheHit is set if a plan from the query plan cache was used (and
 	// re-optimized).
 	planFlagOptCacheHit