Skip to content

Commit

Permalink
execagg: extract out some aggregate helpers into a separate package
Browse files Browse the repository at this point in the history
This allows us to break dependency of `colexecagg` on `execinfra`.

Release note: None
  • Loading branch information
yuzefovich committed Apr 29, 2022
1 parent dd5d353 commit c1dd4c3
Show file tree
Hide file tree
Showing 19 changed files with 55 additions and 32 deletions.
1 change: 1 addition & 0 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ go_library(
"//pkg/sql/distsql",
"//pkg/sql/enum",
"//pkg/sql/execinfra",
"//pkg/sql/execinfra/execagg",
"//pkg/sql/execinfra/execopnode",
"//pkg/sql/execinfrapb",
"//pkg/sql/execstats",
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/colexec/colexecagg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ go_library(
"//pkg/sql/colexecerror",
"//pkg/sql/colexecop",
"//pkg/sql/colmem",
"//pkg/sql/execinfra",
"//pkg/sql/execinfra/execagg",
"//pkg/sql/execinfrapb",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/tree",
Expand Down Expand Up @@ -83,5 +83,6 @@ disallowed_imports_test(
"//pkg/sql/colexec/colexecprojconst",
"//pkg/sql/colexec/colexecsel",
"//pkg/sql/colexec/colexecwindow",
"//pkg/sql/execinfra",
],
)
8 changes: 4 additions & 4 deletions pkg/sql/colexec/colexecagg/aggregate_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/colexecerror"
"github.com/cockroachdb/cockroach/pkg/sql/colexecop"
"github.com/cockroachdb/cockroach/pkg/sql/colmem"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
Expand Down Expand Up @@ -448,16 +448,16 @@ func ProcessAggregations(
aggregations []execinfrapb.AggregatorSpec_Aggregation,
inputTypes []*types.T,
) (
constructors []execinfra.AggregateConstructor,
constructors []execagg.AggregateConstructor,
constArguments []tree.Datums,
outputTypes []*types.T,
err error,
) {
constructors = make([]execinfra.AggregateConstructor, len(aggregations))
constructors = make([]execagg.AggregateConstructor, len(aggregations))
constArguments = make([]tree.Datums, len(aggregations))
outputTypes = make([]*types.T, len(aggregations))
for i, aggFn := range aggregations {
constructors[i], constArguments[i], outputTypes[i], err = execinfra.GetAggregateConstructor(
constructors[i], constArguments[i], outputTypes[i], err = execagg.GetAggregateConstructor(
evalCtx, semaCtx, &aggFn, inputTypes,
)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/colexec/colexecagg/aggregators_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ package colexecagg
import (
"github.com/cockroachdb/cockroach/pkg/sql/colexecop"
"github.com/cockroachdb/cockroach/pkg/sql/colmem"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
Expand All @@ -32,7 +32,7 @@ type NewAggregatorArgs struct {
InputTypes []*types.T
Spec *execinfrapb.AggregatorSpec
EvalCtx *eval.Context
Constructors []execinfra.AggregateConstructor
Constructors []execagg.AggregateConstructor
ConstArguments []tree.Datums
OutputTypes []*types.T
}
6 changes: 3 additions & 3 deletions pkg/sql/colexec/colexecagg/default_agg_tmpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/colexecerror"
"github.com/cockroachdb/cockroach/pkg/sql/colexecop"
"github.com/cockroachdb/cockroach/pkg/sql/colmem"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
Expand Down Expand Up @@ -118,7 +118,7 @@ func (a *default_AGGKINDAgg) Reset() {

func newDefault_AGGKINDAggAlloc(
allocator *colmem.Allocator,
constructor execinfra.AggregateConstructor,
constructor execagg.AggregateConstructor,
evalCtx *eval.Context,
inputArgsConverter *colconv.VecToDatumConverter,
numArguments int,
Expand Down Expand Up @@ -148,7 +148,7 @@ type default_AGGKINDAggAlloc struct {
aggAllocBase
aggFuncs []default_AGGKINDAgg

constructor execinfra.AggregateConstructor
constructor execagg.AggregateConstructor
evalCtx *eval.Context
// inputArgsConverter is a converter from coldata.Vecs to tree.Datums that
// is shared among all aggregate functions and is managed by the aggregator
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/colexec/colexecagg/hash_default_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions pkg/sql/colexec/colexecagg/ordered_default_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/sql/distsql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ go_test(
"//pkg/sql/colflow",
"//pkg/sql/colmem",
"//pkg/sql/execinfra",
"//pkg/sql/execinfra/execagg",
"//pkg/sql/execinfrapb",
"//pkg/sql/flowinfra",
"//pkg/sql/randgen",
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/distsql/columnar_operators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecagg"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexectestutils"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecwindow"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/randgen"
"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
Expand Down Expand Up @@ -275,7 +275,7 @@ func TestAggregatorAgainstProcessor(t *testing.T) {
for _, typ := range aggFnInputTypes {
hasJSONColumn = hasJSONColumn || typ.Family() == types.JsonFamily
}
if _, outputType, err := execinfra.GetAggregateInfo(aggFn, aggFnInputTypes...); err == nil {
if _, outputType, err := execagg.GetAggregateInfo(aggFn, aggFnInputTypes...); err == nil {
outputTypes[i] = outputType
break
}
Expand Down Expand Up @@ -1202,7 +1202,7 @@ func TestWindowFunctionsAgainstProcessor(t *testing.T) {
}
windowerSpec.WindowFns[0].Frame = generateWindowFrame(t, rng, &ordering, inputTypes)

_, outputType, err := execinfra.GetWindowFunctionInfo(fun, argTypes...)
_, outputType, err := execagg.GetWindowFunctionInfo(fun, argTypes...)
require.NoError(t, err)
pspec := &execinfrapb.ProcessorSpec{
Input: []execinfrapb.InputSyncSpec{{ColumnTypes: inputTypes}},
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/colflow"
"github.com/cockroachdb/cockroach/pkg/sql/distsql"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execopnode"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/execstats"
Expand Down Expand Up @@ -1936,7 +1937,7 @@ func (dsp *DistSQLPlanner) planAggregators(
for j, c := range e.ColIdx {
argTypes[j] = inputTypes[c]
}
_, outputType, err := execinfra.GetAggregateInfo(localFunc, argTypes...)
_, outputType, err := execagg.GetAggregateInfo(localFunc, argTypes...)
if err != nil {
return err
}
Expand Down Expand Up @@ -1988,7 +1989,7 @@ func (dsp *DistSQLPlanner) planAggregators(
// the current aggregation e.
argTypes[i] = intermediateTypes[argIdxs[i]]
}
_, outputType, err := execinfra.GetAggregateInfo(finalInfo.Fn, argTypes...)
_, outputType, err := execagg.GetAggregateInfo(finalInfo.Fn, argTypes...)
if err != nil {
return err
}
Expand Down Expand Up @@ -2143,7 +2144,7 @@ func (dsp *DistSQLPlanner) planAggregators(
}
copy(argTypes[len(agg.ColIdx):], info.argumentsColumnTypes[i])
var err error
_, returnTyp, err := execinfra.GetAggregateInfo(agg.Func, argTypes...)
_, returnTyp, err := execagg.GetAggregateInfo(agg.Func, argTypes...)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/distsql_plan_window.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
package sql

import (
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/rowexec"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
Expand Down Expand Up @@ -110,7 +110,7 @@ func (s *windowPlanState) createWindowFnSpec(
for i, argIdx := range funcInProgress.argsIdxs {
argTypes[i] = s.plan.GetResultTypes()[argIdx]
}
_, outputType, err := execinfra.GetWindowFunctionInfo(funcSpec, argTypes...)
_, outputType, err := execagg.GetWindowFunctionInfo(funcSpec, argTypes...)
if err != nil {
return execinfrapb.WindowerSpec_WindowFn{}, outputType, err
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/sql/execinfra/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ load("//pkg/testutils/buildutil:buildutil.bzl", "disallowed_imports_test")
go_library(
name = "execinfra",
srcs = [
"aggregatorbase.go",
"base.go",
"flow_context.go",
"metadata_test_receiver.go",
Expand Down Expand Up @@ -48,7 +47,6 @@ go_library(
"//pkg/sql/rowenc",
"//pkg/sql/rowenc/valueside",
"//pkg/sql/rowinfra",
"//pkg/sql/sem/builtins",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/tree",
"//pkg/sql/sessiondata",
Expand Down
16 changes: 16 additions & 0 deletions pkg/sql/execinfra/execagg/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "execagg",
srcs = ["base.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg",
visibility = ["//visibility:public"],
deps = [
"//pkg/sql/execinfrapb",
"//pkg/sql/sem/builtins",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/tree",
"//pkg/sql/types",
"@com_github_cockroachdb_errors//:errors",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package execinfra
package execagg

import (
"strings"
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/physicalplan/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ go_test(
"//pkg/sql/catalog/desctestutils",
"//pkg/sql/distsql",
"//pkg/sql/execinfra",
"//pkg/sql/execinfra/execagg",
"//pkg/sql/execinfrapb",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/physicalplan/replicaoracle",
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/physicalplan/aggregator_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils"
"github.com/cockroachdb/cockroach/pkg/sql/distsql"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
Expand Down Expand Up @@ -188,7 +189,7 @@ func checkDistAggregationInfo(
intermediaryTypes := make([]*types.T, numIntermediary)
for i, fn := range info.LocalStage {
var err error
_, returnTyp, err := execinfra.GetAggregateInfo(fn, colTypes...)
_, returnTyp, err := execagg.GetAggregateInfo(fn, colTypes...)
if err != nil {
t.Fatal(err)
}
Expand All @@ -207,7 +208,7 @@ func checkDistAggregationInfo(
inputTypes[i] = intermediaryTypes[localIdx]
}
var err error
_, finalOutputTypes[i], err = execinfra.GetAggregateInfo(finalInfo.Fn, inputTypes...)
_, finalOutputTypes[i], err = execagg.GetAggregateInfo(finalInfo.Fn, inputTypes...)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -509,7 +510,7 @@ func TestSingleArgumentDistAggregateFunctions(t *testing.T) {
continue
}
// See if this column works with this function.
_, _, err := execinfra.GetAggregateInfo(fn, col.GetType())
_, _, err := execagg.GetAggregateInfo(fn, col.GetType())
if err != nil {
continue
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/rowexec/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ go_library(
"//pkg/sql/catalog/descs",
"//pkg/sql/catalog/typedesc",
"//pkg/sql/execinfra",
"//pkg/sql/execinfra/execagg",
"//pkg/sql/execinfra/execopnode",
"//pkg/sql/execinfra/execreleasable",
"//pkg/sql/execinfrapb",
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/rowexec/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"unsafe"

"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execopnode"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/execstats"
Expand Down Expand Up @@ -133,7 +134,7 @@ func (ag *aggregatorBase) init(
)
}
}
constructor, arguments, outputType, err := execinfra.GetAggregateConstructor(
constructor, arguments, outputType, err := execagg.GetAggregateConstructor(
flowCtx.EvalCtx, semaCtx, &aggInfo, ag.inputTypes,
)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/rowexec/windower.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"context"

"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execagg"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra/execopnode"
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/execstats"
Expand Down Expand Up @@ -125,7 +126,7 @@ func newWindower(
for i, argIdx := range windowFn.ArgsIdxs {
argTypes[i] = w.inputTypes[argIdx]
}
windowConstructor, outputType, err := execinfra.GetWindowFunctionInfo(windowFn.Func, argTypes...)
windowConstructor, outputType, err := execagg.GetWindowFunctionInfo(windowFn.Func, argTypes...)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit c1dd4c3

Please sign in to comment.