execgen: some small improvements #49727
Conversation
|
This change is |
There was a problem hiding this comment.
Nice!
Reviewed 22 of 22 files at r1, 24 of 24 files at r2.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis)
pkg/col/coldata/vec_tmpl.go, line 38 at r1 (raw file):
// Dummy import to pull in "typeconv" package. var _ = typeconv.DatumVecCanonicalTypeFamily
This can also be removed I think (and in a few other templates where we have this line).
pkg/sql/colexec/execgen/cmd/execgen/distinct_gen.go, line 32 at r2 (raw file):
r := strings.NewReplacer( "_CANONICAL_TYPE_FAMILY", "{{.CanonicalTypeFamilyStr}}", "_CANONICAL_TYPE_FAMILYY", "{{.CanonicalTypeFamilyStr}}",
An artifact.
pkg/sql/colexec/execgen/cmd/execgen/vec_comparators_gen.go, line 31 at r2 (raw file):
r := strings.NewReplacer( "_CANONICAL_TYPE_FAMILY", "{{.CanonicalTypeFamilyStr}}", "_TYPE_WIDTH", "{{.Width}}{{if eq .Width -1}}: default{{end}}",
nit: this could be "_TYPE_WIDTH", typeWidthReplacement,.
jordanlewis
force-pushed
the
imports-and-replacer
branch
from
4ffdfe6
to
9f59b87
Compare
Remove the Dummy import plague on the template files by using the `goimports` infrastructure to make the .eg.go files get their imports at generation time. Release note: None
instead of repeated strings.ReplaceAll Release note: None
jordanlewis
force-pushed
the
imports-and-replacer
branch
from
9f59b87
to
2735048
Compare
|
bors r+ |
There was a problem hiding this comment.
Reviewable status:
pkg/col/coldata/vec_tmpl.go, line 38 at r1 (raw file):
Previously, yuzefovich wrote…
This can also be removed I think (and in a few other templates where we have this line).
Done.
pkg/sql/colexec/execgen/cmd/execgen/distinct_gen.go, line 32 at r2 (raw file):
Previously, yuzefovich wrote…
An artifact.
Done.
Build failed |
|
bors r+ |
Build succeeded |
importsto avoid having to precisely specify template importsstrings.Replacerto streamline some code