Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: remove vectorize=201auto option #55907

Merged
merged 1 commit into from
Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 8 additions & 41 deletions pkg/sql/colexec/colbuilder/execplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,8 @@ func needHashAggregator(aggSpec *execinfrapb.AggregatorSpec) (bool, error) {
// isSupported checks whether we have a columnar operator equivalent to a
// processor described by spec. Note that it doesn't perform any other checks
// (like validity of the number of inputs).
func isSupported(mode sessiondatapb.VectorizeExecMode, spec *execinfrapb.ProcessorSpec) error {
func isSupported(spec *execinfrapb.ProcessorSpec) error {
core := spec.Core
isFullVectorization := mode == sessiondatapb.VectorizeOn ||
mode == sessiondatapb.VectorizeExperimentalAlways

switch {
case core.Noop != nil:
return nil
Expand Down Expand Up @@ -190,11 +187,6 @@ func isSupported(mode sessiondatapb.VectorizeExecMode, spec *execinfrapb.Process
if core.Distinct.ErrorOnDup != "" {
return errors.Newf("distinct with error on duplicates not supported")
}
if !isFullVectorization {
if len(core.Distinct.OrderedColumns) < len(core.Distinct.DistinctColumns) {
return errors.Newf("unordered distinct can only run in vectorize 'on' mode")
}
}
return nil

case core.Ordinality != nil:
Expand Down Expand Up @@ -246,12 +238,6 @@ func isSupported(mode sessiondatapb.VectorizeExecMode, spec *execinfrapb.Process
if _, supported := SupportedWindowFns[*wf.Func.WindowFunc]; !supported {
return errors.Newf("window function %s is not supported", wf.String())
}
if !isFullVectorization {
switch *wf.Func.WindowFunc {
case execinfrapb.WindowerSpec_PERCENT_RANK, execinfrapb.WindowerSpec_CUME_DIST:
return errors.Newf("window function %s can only run in vectorize 'on' mode", wf.String())
}
}
}
return nil

Expand Down Expand Up @@ -431,8 +417,6 @@ func (r opResult) createAndWrapRowSource(
}
if spec.Core.JoinReader == nil {
switch flowCtx.EvalCtx.SessionData.VectorizeMode {
case sessiondatapb.Vectorize201Auto:
return errors.New("rowexec processor wrapping for non-JoinReader core unsupported in vectorize=201auto mode")
case sessiondatapb.VectorizeExperimentalAlways:
return causeToWrap
}
Expand Down Expand Up @@ -477,11 +461,7 @@ func (r opResult) createAndWrapRowSource(
if err != nil {
return err
}
// We say that the wrapped processor is "streaming" because it is not a
// buffering operator (even if it is a buffering processor). This is not a
// problem for memory accounting because each processor does that on its
// own, so the used memory will be accounted for.
r.Op, r.IsStreaming = c, true
r.Op = c
r.MetadataSources = append(r.MetadataSources, c)
r.ToClose = append(r.ToClose, c)
return nil
Expand Down Expand Up @@ -551,18 +531,11 @@ func NewColOperator(
core := &spec.Core
post := &spec.Post

// By default, we safely assume that an operator is not streaming. Note that
// projections, renders, filters, limits, offsets as well as all internal
// operators (like stats collectors and cancel checkers) are streaming, so in
// order to determine whether the resulting chain of operators is streaming,
// it is sufficient to look only at the "core" operator.
result.IsStreaming = false

// resultPreSpecPlanningStateShallowCopy is a shallow copy of the result
// before any specs are planned. Used if there is a need to backtrack.
resultPreSpecPlanningStateShallowCopy := r

if err = isSupported(flowCtx.EvalCtx.SessionData.VectorizeMode, spec); err != nil {
if err = isSupported(spec); err != nil {
// We refuse to wrap LocalPlanNode processor (which is a DistSQL wrapper
// around a planNode) because it creates complications, and a flow with
// such processor probably will not benefit from the vectorization.
Expand Down Expand Up @@ -620,7 +593,7 @@ func NewColOperator(
if err := checkNumIn(inputs, 1); err != nil {
return r, err
}
result.Op, result.IsStreaming = colexec.NewNoop(inputs[0]), true
result.Op = colexec.NewNoop(inputs[0])
result.ColumnTypes = make([]*types.T, len(spec.Input[0].ColumnTypes))
copy(result.ColumnTypes, spec.Input[0].ColumnTypes)

Expand All @@ -631,7 +604,7 @@ func NewColOperator(
if core.Values.NumRows != 0 {
return r, errors.AssertionFailedf("values core only with zero rows supported, %d given", core.Values.NumRows)
}
result.Op, result.IsStreaming = colexec.NewZeroOpNoInput(), true
result.Op = colexec.NewZeroOpNoInput()
result.ColumnTypes = make([]*types.T, len(core.Values.Columns))
for i, col := range core.Values.Columns {
result.ColumnTypes[i] = col.Type
Expand All @@ -645,7 +618,7 @@ func NewColOperator(
if err != nil {
return r, err
}
result.Op, result.IsStreaming = scanOp, true
result.Op = scanOp
result.IOReader = scanOp
result.MetadataSources = append(result.MetadataSources, scanOp)
// colBatchScan is wrapped with a cancel checker below, so we need to
Expand Down Expand Up @@ -675,13 +648,13 @@ func NewColOperator(
// TODO(solon): The distsql plan for this case includes a TableReader, so
// we end up creating an orphaned colBatchScan. We should avoid that.
// Ideally the optimizer would not plan a scan in this unusual case.
result.Op, result.IsStreaming, err = colexec.NewSingleTupleNoInputOp(streamingAllocator), true, nil
result.Op, err = colexec.NewSingleTupleNoInputOp(streamingAllocator), nil
// We make ColumnTypes non-nil so that sanity check doesn't panic.
result.ColumnTypes = []*types.T{}
break
}
if aggSpec.IsRowCount() {
result.Op, result.IsStreaming, err = colexec.NewCountOp(streamingAllocator, inputs[0]), true, nil
result.Op, err = colexec.NewCountOp(streamingAllocator, inputs[0]), nil
result.ColumnTypes = []*types.T{types.Int}
break
}
Expand Down Expand Up @@ -727,7 +700,6 @@ func NewColOperator(
streamingAllocator, streamingMemAccount, inputs[0], inputTypes, aggSpec,
evalCtx, constructors, constArguments, result.ColumnTypes, aggSpec.IsScalar(),
)
result.IsStreaming = true
}
result.ToClose = append(result.ToClose, result.Op.(colexecbase.Closer))

Expand All @@ -739,7 +711,6 @@ func NewColOperator(
copy(result.ColumnTypes, spec.Input[0].ColumnTypes)
if len(core.Distinct.OrderedColumns) == len(core.Distinct.DistinctColumns) {
result.Op, err = colexec.NewOrderedDistinct(inputs[0], core.Distinct.OrderedColumns, result.ColumnTypes)
result.IsStreaming = true
} else {
distinctMemAccount := streamingMemAccount
if !useStreamingMemAccountForBuffering {
Expand Down Expand Up @@ -768,7 +739,6 @@ func NewColOperator(
}
outputIdx := len(spec.Input[0].ColumnTypes)
result.Op = colexec.NewOrdinalityOp(streamingAllocator, inputs[0], outputIdx)
result.IsStreaming = true
result.ColumnTypes = appendOneType(spec.Input[0].ColumnTypes, types.Int)

case core.HashJoiner != nil:
Expand Down Expand Up @@ -887,9 +857,6 @@ func NewColOperator(
if err := checkNumIn(inputs, 2); err != nil {
return r, err
}
// Merge joiner is a streaming operator when equality columns form a key
// for both of the inputs.
result.IsStreaming = core.MergeJoiner.LeftEqColumnsAreKey && core.MergeJoiner.RightEqColumnsAreKey

leftTypes := make([]*types.T, len(spec.Input[0].ColumnTypes))
copy(leftTypes, spec.Input[0].ColumnTypes)
Expand Down
7 changes: 3 additions & 4 deletions pkg/sql/colexec/op_creation.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ type NewColOperatorResult struct {
InternalMemUsage int
MetadataSources []execinfrapb.MetadataSource
// ToClose is a slice of components that need to be Closed.
ToClose []colexecbase.Closer
IsStreaming bool
OpMonitors []*mon.BytesMonitor
OpAccounts []*mon.BoundAccount
ToClose []colexecbase.Closer
OpMonitors []*mon.BytesMonitor
OpAccounts []*mon.BoundAccount
}
1 change: 0 additions & 1 deletion pkg/sql/colflow/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ go_library(
"//pkg/sql/pgwire/pgcode",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/rowexec",
"//pkg/sql/sessiondatapb",
"//pkg/sql/types",
"//pkg/util",
"//pkg/util/log",
Expand Down
14 changes: 0 additions & 14 deletions pkg/sql/colflow/vectorized_flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/rowexec"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -1034,11 +1033,6 @@ func (s *vectorizedFlowCreator) setupFlow(
if flowCtx.EvalCtx.SessionData.TestingVectorizeInjectPanics {
result.Op = colexec.NewPanicInjector(result.Op)
}
if flowCtx.EvalCtx.SessionData.VectorizeMode == sessiondatapb.Vectorize201Auto &&
!result.IsStreaming {
err = errors.Errorf("non-streaming operator encountered when vectorize=201auto")
return
}
// We created a streaming memory account when calling NewColOperator above,
// so there is definitely at least one memory account, and it doesn't
// matter which one we grow.
Expand Down Expand Up @@ -1076,14 +1070,6 @@ func (s *vectorizedFlowCreator) setupFlow(
}
}

if (flowCtx.EvalCtx.SessionData.VectorizeMode == sessiondatapb.Vectorize201Auto) &&
pspec.Output[0].Type == execinfrapb.OutputRouterSpec_BY_HASH {
// colexec.HashRouter is not supported when vectorize=auto since it can
// buffer an unlimited number of tuples, even though it falls back to
// disk. vectorize=on does support this.
err = errors.Errorf("hash router encountered when vectorize=201auto")
return
}
if err = s.setupOutput(
ctx, flowCtx, pspec, op, result.ColumnTypes, metadataSourcesQueue, toClose, factory,
); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/distsql_running.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (dsp *DistSQLPlanner) setupFlows(
}

if evalCtx.SessionData.VectorizeMode != sessiondatapb.VectorizeOff {
if !vectorizeThresholdMet && (evalCtx.SessionData.VectorizeMode == sessiondatapb.Vectorize201Auto || evalCtx.SessionData.VectorizeMode == sessiondatapb.VectorizeOn) {
if !vectorizeThresholdMet && evalCtx.SessionData.VectorizeMode == sessiondatapb.VectorizeOn {
// Vectorization is not justified for this flow because the expected
// amount of data is too small and the overhead of pre-allocating data
// structures needed for the vectorized engine is expected to dominate
Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,8 @@ var VectorizeClusterMode = settings.RegisterEnumSetting(
"default vectorize mode",
"on",
map[int64]string{
int64(sessiondatapb.VectorizeOff): "off",
int64(sessiondatapb.Vectorize201Auto): "201auto",
int64(sessiondatapb.VectorizeOn): "on",
int64(sessiondatapb.VectorizeOff): "off",
int64(sessiondatapb.VectorizeOn): "on",
},
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/explain_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func explainGetDistributedAndVectorized(
vectorizedThresholdMet := physicalPlan.MaxEstimatedRowCount >= ctxSessionData.VectorizeRowCountThreshold
if ctxSessionData.VectorizeMode == sessiondatapb.VectorizeOff {
willVectorize = false
} else if !vectorizedThresholdMet && (ctxSessionData.VectorizeMode == sessiondatapb.Vectorize201Auto || ctxSessionData.VectorizeMode == sessiondatapb.VectorizeOn) {
} else if !vectorizedThresholdMet && ctxSessionData.VectorizeMode == sessiondatapb.VectorizeOn {
willVectorize = false
} else {
willVectorize = true
Expand Down
45 changes: 0 additions & 45 deletions pkg/sql/logictest/logic.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,12 +489,6 @@ var logicTestConfigs = []testClusterConfig{
binaryVersion: roachpb.Version{Major: 1, Minor: 1},
disableUpgrade: true,
},
{
name: "local-vec-auto",
numNodes: 1,
overrideAutoStats: "false",
overrideVectorize: "201auto",
},
{
name: "local-mixed-19.2-20.1",
numNodes: 1,
Expand Down Expand Up @@ -535,24 +529,6 @@ var logicTestConfigs = []testClusterConfig{
overrideAutoStats: "false",
overrideVectorize: "off",
},
{
name: "fakedist-vec-auto",
numNodes: 3,
useFakeSpanResolver: true,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
},
{
name: "fakedist-vec-auto-disk",
numNodes: 3,
useFakeSpanResolver: true,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
sqlExecUseDisk: true,
skipShort: true,
},
{
name: "fakedist-metadata",
numNodes: 3,
Expand Down Expand Up @@ -585,22 +561,6 @@ var logicTestConfigs = []testClusterConfig{
overrideDistSQLMode: "on",
overrideAutoStats: "false",
},
{
name: "5node-vec-auto",
numNodes: 5,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
},
{
name: "5node-vec-disk-auto",
numNodes: 5,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
sqlExecUseDisk: true,
skipShort: true,
},
{
name: "5node-metadata",
numNodes: 5,
Expand Down Expand Up @@ -733,12 +693,9 @@ var (
defaultConfigNames = []string{
"local",
"local-vec-off",
"local-vec-auto",
"local-spec-planning",
"fakedist",
"fakedist-vec-off",
"fakedist-vec-auto",
"fakedist-vec-auto-disk",
"fakedist-metadata",
"fakedist-disk",
"fakedist-spec-planning",
Expand All @@ -747,8 +704,6 @@ var (
fiveNodeDefaultConfigName = "5node-default-configs"
fiveNodeDefaultConfigNames = []string{
"5node",
"5node-vec-auto",
"5node-vec-disk-auto",
"5node-metadata",
"5node-disk",
"5node-spec-planning",
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/column_families
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# LogicTest: local local-vec-auto 3node-tenant
# LogicTest: local 3node-tenant

# Test that different operations still succeed when the primary key is not in column family 0.

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/explain
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# LogicTest: local local-vec-off local-vec-auto local-spec-planning
# LogicTest: local local-vec-off local-spec-planning

statement ok
CREATE TABLE t (a INT PRIMARY KEY)
Expand Down
20 changes: 0 additions & 20 deletions pkg/sql/logictest/testdata/logic_test/merge_join_192

This file was deleted.

3 changes: 0 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/set
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,6 @@ SET distsql = off
statement error invalid value for parameter "distsql": "bogus"
SET distsql = bogus

statement ok
SET vectorize = '201auto'

statement ok
SET vectorize = on

Expand Down
4 changes: 0 additions & 4 deletions pkg/sql/sessiondatapb/session_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ func (m VectorizeExecMode) String() string {
switch m {
case VectorizeOff:
return "off"
case Vectorize201Auto:
return "201auto"
case VectorizeOn:
return "on"
case VectorizeExperimentalAlways:
Expand All @@ -100,8 +98,6 @@ func VectorizeExecModeFromString(val string) (VectorizeExecMode, bool) {
switch strings.ToUpper(val) {
case "OFF":
m = VectorizeOff
case "201AUTO":
m = Vectorize201Auto
case "ON":
m = VectorizeOn
case "EXPERIMENTAL_ALWAYS":
Expand Down
Loading