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

colexecproj: break it down into two packages #79462

Merged
merged 2 commits into from
Apr 8, 2022
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
9 changes: 5 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -865,11 +865,12 @@ EXECGEN_TARGETS = \
pkg/sql/colexec/colexecjoin/mergejoiner_rightanti.eg.go \
pkg/sql/colexec/colexecjoin/mergejoiner_rightouter.eg.go \
pkg/sql/colexec/colexecjoin/mergejoiner_rightsemi.eg.go \
pkg/sql/colexec/colexecproj/default_cmp_proj_ops.eg.go \
pkg/sql/colexec/colexecproj/proj_const_left_ops.eg.go \
pkg/sql/colexec/colexecproj/proj_const_right_ops.eg.go \
pkg/sql/colexec/colexecproj/proj_like_ops.eg.go \
pkg/sql/colexec/colexecproj/default_cmp_proj_op.eg.go \
pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go \
pkg/sql/colexec/colexecprojconst/default_cmp_proj_const_op.eg.go \
pkg/sql/colexec/colexecprojconst/proj_const_left_ops.eg.go \
pkg/sql/colexec/colexecprojconst/proj_const_right_ops.eg.go \
pkg/sql/colexec/colexecprojconst/proj_like_ops.eg.go \
pkg/sql/colexec/colexecsel/default_cmp_sel_ops.eg.go \
pkg/sql/colexec/colexecsel/selection_ops.eg.go \
pkg/sql/colexec/colexecsel/sel_like_ops.eg.go \
Expand Down
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ ALL_TESTS = [
"//pkg/sql/colexec/colexecjoin:colexecjoin_test",
"//pkg/sql/colexec/colexecproj:colexecproj_disallowed_imports_test",
"//pkg/sql/colexec/colexecproj:colexecproj_test",
"//pkg/sql/colexec/colexecprojconst:colexecprojconst_disallowed_imports_test",
"//pkg/sql/colexec/colexecprojconst:colexecprojconst_test",
"//pkg/sql/colexec/colexecsel:colexecsel_disallowed_imports_test",
"//pkg/sql/colexec/colexecsel:colexecsel_test",
"//pkg/sql/colexec/colexecspan:colexecspan_disallowed_imports_test",
Expand Down
9 changes: 5 additions & 4 deletions pkg/gen/execgen.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ EXECGEN_SRCS = [
"//pkg/sql/colexec/colexecjoin:mergejoiner_rightanti.eg.go",
"//pkg/sql/colexec/colexecjoin:mergejoiner_rightouter.eg.go",
"//pkg/sql/colexec/colexecjoin:mergejoiner_rightsemi.eg.go",
"//pkg/sql/colexec/colexecproj:default_cmp_proj_ops.eg.go",
"//pkg/sql/colexec/colexecproj:proj_const_left_ops.eg.go",
"//pkg/sql/colexec/colexecproj:proj_const_right_ops.eg.go",
"//pkg/sql/colexec/colexecproj:proj_like_ops.eg.go",
"//pkg/sql/colexec/colexecproj:default_cmp_proj_op.eg.go",
"//pkg/sql/colexec/colexecproj:proj_non_const_ops.eg.go",
"//pkg/sql/colexec/colexecprojconst:default_cmp_proj_const_op.eg.go",
"//pkg/sql/colexec/colexecprojconst:proj_const_left_ops.eg.go",
"//pkg/sql/colexec/colexecprojconst:proj_const_right_ops.eg.go",
"//pkg/sql/colexec/colexecprojconst:proj_like_ops.eg.go",
"//pkg/sql/colexec/colexecsel:default_cmp_sel_ops.eg.go",
"//pkg/sql/colexec/colexecsel:sel_like_ops.eg.go",
"//pkg/sql/colexec/colexecsel:selection_ops.eg.go",
Expand Down
16 changes: 16 additions & 0 deletions pkg/sql/colexec/COLEXEC.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,19 @@ $(location :goimports) -w $@
tools = [":execgen", ":goimports"],
visibility = [":__pkg__", "//pkg/gen:__pkg__"],
)

# Generating the code for `default_cmp_proj_const_op.eg.go` requires special
# handling because the template lives in a different package.
def gen_default_cmp_proj_const_rule(name, target, visibility = ["//visibility:private"]):
native.genrule(
name = name,
srcs = ["//pkg/sql/colexec/colexecproj:default_cmp_proj_ops_tmpl.go"],
outs = [target],
cmd = """\
export COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=true
$(location :execgen) -template $(SRCS) -fmt=false pkg/sql/colexec/colexecprojconst/$@ > $@
$(location :goimports) -w $@
""",
tools = [":execgen", ":goimports"],
visibility = [":__pkg__", "//pkg/gen:__pkg__"],
)
1 change: 1 addition & 0 deletions pkg/sql/colexec/colbuilder/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ go_library(
"//pkg/sql/colexec/colexecbase",
"//pkg/sql/colexec/colexecjoin",
"//pkg/sql/colexec/colexecproj",
"//pkg/sql/colexec/colexecprojconst",
"//pkg/sql/colexec/colexecsel",
"//pkg/sql/colexec/colexecutils",
"//pkg/sql/colexec/colexecwindow",
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/colexec/colbuilder/execplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecbase"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecjoin"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecprojconst"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecsel"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecutils"
"github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecwindow"
Expand Down Expand Up @@ -2335,7 +2336,7 @@ func planProjectionExpr(
resultIdx = len(typs)
// The projection result will be outputted to a new column which is
// appended to the input batch.
op, err = colexecproj.GetProjectionLConstOperator(
op, err = colexecprojconst.GetProjectionLConstOperator(
allocator, typs, left.ResolvedType(), outputType, projOp, input,
rightIdx, lConstArg, resultIdx, evalCtx, binFn, cmpExpr,
)
Expand Down Expand Up @@ -2381,7 +2382,7 @@ func planProjectionExpr(
switch cmpProjOp.Symbol {
case treecmp.Like, treecmp.NotLike:
negate := cmpProjOp.Symbol == treecmp.NotLike
op, err = colexecproj.GetLikeProjectionOperator(
op, err = colexecprojconst.GetLikeProjectionOperator(
allocator, evalCtx, input, leftIdx, resultIdx,
string(tree.MustBeDString(rConstArg)), negate,
)
Expand Down Expand Up @@ -2412,7 +2413,7 @@ func planProjectionExpr(
if op == nil || err != nil {
// op hasn't been created yet, so let's try the constructor for
// all other projection operators.
op, err = colexecproj.GetProjectionRConstOperator(
op, err = colexecprojconst.GetProjectionRConstOperator(
allocator, typs, right.ResolvedType(), outputType, projOp,
input, leftIdx, rConstArg, resultIdx, evalCtx, binFn, cmpExpr,
)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/colexec/colexecagg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ disallowed_imports_test(
"//pkg/sql/colexec/colexechash",
"//pkg/sql/colexec/colexecjoin",
"//pkg/sql/colexec/colexecproj",
"//pkg/sql/colexec/colexecprojconst",
"//pkg/sql/colexec/colexecsel",
"//pkg/sql/colexec/colexecwindow",
],
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/colexec/colexecbase/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ disallowed_imports_test(
"//pkg/sql/colexec/colexechash",
"//pkg/sql/colexec/colexecjoin",
"//pkg/sql/colexec/colexecproj",
"//pkg/sql/colexec/colexecprojconst",
"//pkg/sql/colexec/colexecsel",
"//pkg/sql/colexec/colexecwindow",
],
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/colexec/colexechash/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ disallowed_imports_test(
"//pkg/sql/colexec/colexecbase",
"//pkg/sql/colexec/colexecjoin",
"//pkg/sql/colexec/colexecproj",
"//pkg/sql/colexec/colexecprojconst",
"//pkg/sql/colexec/colexecsel",
"//pkg/sql/colexec/colexecwindow",
],
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/colexec/colexecjoin/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ disallowed_imports_test(
"//pkg/sql/colexec",
"//pkg/sql/colexec/colexecagg",
"//pkg/sql/colexec/colexecproj",
"//pkg/sql/colexec/colexecprojconst",
"//pkg/sql/colexec/colexecsel",
"//pkg/sql/colexec/colexecwindow",
],
Expand Down
29 changes: 15 additions & 14 deletions pkg/sql/colexec/colexecproj/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ load("//pkg/testutils/buildutil:buildutil.bzl", "disallowed_imports_test")
go_library(
name = "colexecproj",
srcs = [
"like_ops.go",
":gen-exec", # keep
],
importpath = "github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj",
importpath = "github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj", # keep
visibility = ["//visibility:public"],
deps = [
"//pkg/col/coldata", # keep
Expand All @@ -17,22 +16,22 @@ go_library(
"//pkg/server/telemetry", # keep
"//pkg/sql/colconv", # keep
"//pkg/sql/colexec/colexecbase", # keep
"//pkg/sql/colexec/colexeccmp",
"//pkg/sql/colexec/colexecutils",
"//pkg/sql/colexec/colexeccmp", # keep
"//pkg/sql/colexec/colexecutils", # keep
"//pkg/sql/colexec/execgen", # keep
"//pkg/sql/colexecerror", # keep
"//pkg/sql/colexecop",
"//pkg/sql/colmem",
"//pkg/sql/colexecop", # keep
"//pkg/sql/colmem", # keep
"//pkg/sql/execinfra", # keep
"//pkg/sql/sem/tree",
"//pkg/sql/sem/tree", # keep
"//pkg/sql/sem/tree/treebin", # keep
"//pkg/sql/sem/tree/treecmp", # keep
"//pkg/sql/sqltelemetry", # keep
"//pkg/sql/types",
"//pkg/sql/types", # keep
"//pkg/util/duration", # keep
"//pkg/util/json", # keep
"@com_github_cockroachdb_apd_v3//:apd", # keep
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_errors//:errors", # keep
],
)

Expand All @@ -44,7 +43,7 @@ go_test(
"main_test.go",
"projection_ops_test.go",
],
embed = [":colexecproj"],
embed = [":colexecproj"], # keep
tags = ["no-remote"],
deps = [
"//pkg/col/coldata",
Expand Down Expand Up @@ -74,12 +73,13 @@ go_test(
],
)

# Export the template because it is used by another target in colexecprojconst
# package.
exports_files(["default_cmp_proj_ops_tmpl.go"])

# Map between target name and relevant template.
targets = [
("default_cmp_proj_ops.eg.go", "default_cmp_proj_ops_tmpl.go"),
("proj_const_left_ops.eg.go", "proj_const_ops_tmpl.go"),
("proj_const_right_ops.eg.go", "proj_const_ops_tmpl.go"),
("proj_like_ops.eg.go", "proj_const_ops_tmpl.go"),
("default_cmp_proj_op.eg.go", "default_cmp_proj_ops_tmpl.go"),
("proj_non_const_ops.eg.go", "proj_non_const_ops_tmpl.go"),
]

Expand All @@ -99,6 +99,7 @@ disallowed_imports_test(
"//pkg/sql/colexec/colexecagg",
"//pkg/sql/colexec/colexechash",
"//pkg/sql/colexec/colexecjoin",
"//pkg/sql/colexec/colexecprojconst",
"//pkg/sql/colexec/colexecsel",
"//pkg/sql/colexec/colexecwindow",
],
Expand Down

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

9 changes: 5 additions & 4 deletions pkg/sql/colexec/colexecproj/default_cmp_proj_ops_tmpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
// +build execgen_template

//
// This file is the execgen template for default_cmp_proj_ops.eg.go. It's
// formatted in a special way, so it's both valid Go and a valid text/template
// input. This permits editing this file with editor support.
// This file is the execgen template for default_cmp_proj_op.eg.go and
// default_cmp_proj_const_op.eg.go. It's formatted in a special way, so it's
// both valid Go and a valid text/template input. This permits editing this file
// with editor support.
//
// */}}

Expand All @@ -31,7 +32,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
)

// {{range .}}
// {{define "defaultCmpProjOp"}}

type defaultCmp_KINDProjOp struct {
// {{if .IsRightConst}}
Expand Down
12 changes: 0 additions & 12 deletions pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go

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

12 changes: 0 additions & 12 deletions pkg/sql/colexec/colexecproj/proj_non_const_ops_tmpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,6 @@ func _ASSIGN(_, _, _, _, _, _ interface{}) {

// */}}

// projConstOpBase contains all of the fields for projections with a constant,
// except for the constant itself.
// NOTE: this struct should be declared in proj_const_ops_tmpl.go, but if we do
// so, it'll be redeclared because we execute that template twice. To go
// around the problem we specify it here.
type projConstOpBase struct {
colexecop.OneInputHelper
allocator *colmem.Allocator
colIdx int
outputIdx int
}

// projOpBase contains all of the fields for non-constant projections.
type projOpBase struct {
colexecop.OneInputHelper
Expand Down
Loading