colexec: use new execgen directives on several colexec boolean templates#50030
colexec: use new execgen directives on several colexec boolean templates#50030jordanlewis wants to merge 9 commits intocockroachdb:masterfrom
Conversation
Previously, assigning the results of an inlined func with the := syntax produced invalid Go code. Now, it makes sure to define the result variables in the parent scope before entering the new inlined func scope. Release note: None
This commit adds the // execgen:template<arg1, arg2> syntax for
concrete boolean template variables.
For example:
```
// execgen:inline
// execgen:template<templateParam>
func a (runtimeParam int, templateParam bool) int {
if templateParam {
return runtimeParam + 1
} else {
return runtimeParam - 1
}
}
func main() {
x := a(0, true)
y := a(0, false)
fmt.Println(x, y)
}
```
This code will print `1, -1` when eventually executed, but the
templateParam boolean will be evaluated at code generation time and not
appear in the generated code at all. Two variants of `a()` will be
inlined into their callsites - one with templateParam true, and one with
templateParam false.
Release note: None
Early returns in inlined funcs are implemented using gotos. Release note: None
|
This change is |
Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
4d9d116 to
566ed10
Compare
asubiotto
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, 9 of 9 files at r2, 2 of 2 files at r3, 2 of 2 files at r4, 2 of 2 files at r5, 2 of 2 files at r6, 2 of 2 files at r7, 2 of 2 files at r8, 2 of 2 files at r9.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis)
pkg/sql/colexec/and_or_projection_tmpl.go, line 153 at r6 (raw file):
if leftCol.MaybeHasNulls() { for _, i := range origSel { curIdx = addTupleForRight(curIdx, i, sel, leftNulls, leftColVals, knownResult, true)
Interesting, I thought of collapsing this if/else into one statement and passing in leftNullls == nil as the last argument, but I guess that won't work.
pkg/sql/colexec/and_or_projection_tmpl.go, line 157 at r6 (raw file):
} else { for _, i := range origSel { curIdx = addTupleForRight(curIdx, i, sel, leftNulls, leftColVals, knownResult, false)
Can we not pass in a concrete value for leftNulls?
pkg/sql/colexec/any_not_null_agg_tmpl.go, line 190 at r8 (raw file):
// execgen:inline // execgen:template<hasNulls> func _FIND_ANY_NOT_NULL(a *anyNotNull_TYPEAgg, nulls *coldata.Nulls, i int, hasNulls bool) {
I think you missed the function name rename here
pkg/sql/colexec/hashtable_tmpl.go, line 493 at r5 (raw file):
switch ht.probeMode { case hashTableDefaultProbeMode: nDiffers = checkBody(ht, nToCheck, true, false)
nit: while you're here can you add block comments to the booleans?
pkg/sql/colexec/execgen/cmd/execgen/hashjoiner_gen.go, line 23 at r7 (raw file):
s := inputFileContents distinctCollectRightOuter := makeFunctionRegex("_DISTINCT_COLLECT_PROBE_OUTER", 3)
❤️
Based on #49939.
This PR uses the new // execgen:template and // execgen:inline directives on a bunch of colexec templates that need boolean templating.