Skip to content

Commit

Permalink
Merge #39362
Browse files Browse the repository at this point in the history
39362: opt: remove fallback to heuristic planner r=RaduBerinde a=RaduBerinde

The optimizer is able to handle all statements now; we can remove the
logic to fall back to the heuristic planner.

A couple of things needed fixing:
 - we didn't include `Import` and `ShowBackup` as opaque statements (they
   weren't implementing `CCLOnlyStatement`, which is also fixed).
 - the HP was incorrectly hitting the CTE telemetry counter even when
   there was no `WITH`. In the existing test, the HP was running as
   fallback because of some unimplemented functions.

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
  • Loading branch information
craig[bot] and RaduBerinde committed Aug 7, 2019
2 parents d9060dd + 552c88f commit 9990dc7
Show file tree
Hide file tree
Showing 13 changed files with 75 additions and 199 deletions.
7 changes: 4 additions & 3 deletions pkg/server/updates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ func TestReportUsage(t *testing.T) {
t.Fatal(err)
}
// Try a correlated subquery to check that feature reporting.
if _, err := db.Exec(`SELECT x FROM (VALUES (1)) AS b(x) WHERE EXISTS(SELECT * FROM (VALUES (1)) AS a(x) WHERE a.x = b.x)`); err != nil {
if _, err := db.Exec(`SELECT x FROM (VALUES (1)) AS b(x) WHERE EXISTS(SELECT * FROM (VALUES (1)) AS a(x) WHERE a.x = b.x)`); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -690,19 +690,20 @@ func TestReportUsage(t *testing.T) {

// Although the query is executed 10 times, due to plan caching
// keyed by the SQL text, the planning only occurs once.
// TODO(radu): fix this (#39361).
"sql.plan.ops.cast.string::inet": 1,
"sql.plan.ops.bin.jsonb - string": 1,
"sql.plan.builtins.crdb_internal.force_assertion_error(msg: string) -> int": 1,
"sql.plan.ops.array.ind": 1,
"sql.plan.ops.array.cons": 1,
"sql.plan.ops.array.flatten": 1,
// The CTE counter is exercised by `WITH a AS (SELECT 1) ...`.
"sql.plan.cte": 1,

// The subquery counter is exercised by `(1, 20, 30, 40) = (SELECT ...)`.
"sql.plan.subquery": 1,
// The correlated sq counter is exercised by `WHERE EXISTS ( ... )` above.
"sql.plan.subquery.correlated": 1,
// The CTE counter is exercised by `WITH a AS (SELECT 1) ...`.
"sql.plan.cte": 10,

"unimplemented.#33285.json_object_agg": 10,
"unimplemented.pg_catalog.pg_stat_wal_receiver": 10,
Expand Down
98 changes: 7 additions & 91 deletions pkg/sql/conn_executor_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -756,39 +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)
}
log.VEventf(ctx, 1, "optimizer plan failed (isCorrelated=%t): %v", isCorrelated, err)
if !canFallbackFromOpt(err, optMode, stmt) {
result, err = planner.makeOptimizerPlan(ctx)
if err != nil {
log.VEventf(ctx, 1, "optimizer plan failed: %v", err)
return err
}
planner.curPlan.flags.Set(planFlagOptFallback)
log.VEvent(ctx, 1, "optimizer falls back on heuristic planner")
} else {
log.VEvent(ctx, 2, "optimizer disabled")
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
}

Expand All @@ -812,46 +768,6 @@ func (ex *connExecutor) saveLogicalPlanDescription(
return now.Sub(timeLastSampled) >= period
}

// canFallbackFromOpt returns whether we can fallback on the heuristic planner
// when the optimizer hits an error.
func canFallbackFromOpt(err error, optMode sessiondata.OptimizerMode, stmt *Statement) bool {
if !errors.HasUnimplementedError(err) {
// We only fallback on "feature not supported" errors.
return false
}
if err.Error() == "unimplemented: schema change statement cannot follow a statement that has written in the same transaction" {
// This is a special error generated when SetSystemConfigTrigger fails. If
// we fall back to the heuristic planner, the second call to that method
// succeeds.
// TODO(radu): this will go away very soon when we remove fallback
// altogether.
return false
}

if optMode == sessiondata.OptimizerAlways {
// In Always mode we never fallback, with one exception: SET commands (or
// else we can't switch to another mode).
_, isSetVar := stmt.AST.(*tree.SetVar)
return isSetVar
}

// If the statement is EXPLAIN (OPT), then don't fallback (we want to return
// the error, not show a plan from the heuristic planner).
// TODO(radu): this is hacky and doesn't handle an EXPLAIN (OPT) inside
// a larger query.
if e, ok := stmt.AST.(*tree.Explain); ok {
if opts, err := e.ParseOptions(); err == nil && opts.Mode == tree.ExplainOpt {
return false
}
}

// Never fall back on PREPARE AS OPT PLAN.
if _, ok := stmt.AST.(*tree.CannedOptPlan); ok {
return false
}
return true
}

// execWithDistSQLEngine converts a plan to a distributed SQL physical plan and
// runs it.
// If an error is returned, the connection needs to stop processing queries.
Expand Down
18 changes: 5 additions & 13 deletions pkg/sql/conn_executor_prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,32 +204,24 @@ 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.
return flags, nil
}
log.VEventf(ctx, 1, "optimizer prepare failed: %v", err)
if !canFallbackFromOpt(err, optMode, stmt) {
return 0, err
}
flags.Set(planFlagOptFallback)
log.VEvent(ctx, 1, "prepare falls back on heuristic planner")
} else {
log.VEvent(ctx, 2, "optimizer disabled (prepare)")
return 0, err
}
log.VEvent(ctx, 2, "optimizer disabled (prepare)")

// Fallback on the heuristic planner if the optimizer was not enabled or used:
// create a plan for the statement to figure out the typing, then close the
// plan.
// Fallback on the heuristic planner if the optimizer was not enabled: create
// 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
}

Expand Down
2 changes: 0 additions & 2 deletions pkg/sql/executor_statement_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,6 @@ func (ex *connExecutor) updateOptCounters(planFlags planFlags) {
m := &ex.metrics.EngineMetrics
if planFlags.IsSet(planFlagOptUsed) {
m.SQLOptCount.Inc(1)
} else if planFlags.IsSet(planFlagOptFallback) {
m.SQLOptFallbackCount.Inc(1)
}

if planFlags.IsSet(planFlagOptCacheHit) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/opaque.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,16 @@ func init() {
&tree.ShowFingerprints{},
&tree.Truncate{},

// CCL statements.
// CCL statements (without Export which has an optimizer operator).
&tree.Backup{},
&tree.ShowBackup{},
&tree.Restore{},
&tree.CreateChangefeed{},
&tree.CreateRole{},
&tree.DropRole{},
&tree.GrantRole{},
&tree.RevokeRole{},
&tree.Import{},
} {
typ := optbuilder.OpaqueReadOnly
if tree.CanModifySchema(stmt) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/opt/optbuilder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 5 additions & 9 deletions pkg/sql/opt/optbuilder/scalar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
8 changes: 0 additions & 8 deletions pkg/sql/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -821,14 +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

// planFlagOptFallback is set if the optimizer was enabled but did not support the
// statement.
planFlagOptFallback

// planFlagOptCacheHit is set if a plan from the query plan cache was used (and
// re-optimized).
planFlagOptCacheHit
Expand Down
Loading

0 comments on commit 9990dc7

Please sign in to comment.