colexecwindow: implement first_value, last_value, and nth_value #67764

DrewKimball · 2021-07-19T18:16:20Z

logictest: add testfile to ensure that window functions are vectorized

This commit adds a logic testfile that executes the window functions
currently supported by the vectorized engine with
SET vectorize = experimental_always. This tests that supported
window functions can actually be executed in the vectorized engine.

Release note: None

colexecwindow: add first_value, last_value, and nth_value

This patch adds vectorized implementations of the first_value,
last_value, and nth_value window functions. These functions
return an expression evaluated at the first, last, and nth positions
respectively in the window frame for the current row.

Release note (sql change): first_value, last_value, and nth_value
window functions can now be executed in the vectorized engine. This
allows for faster execution time, and also removes the need for
conversions to and from row format.

colexecwindow: test window framer with different memory limits

Previously, the TestWindowFramer used math.MaxInt64 as the memory
limit for the SpillingBuffer used to buffer all tuples in a partition.
This patch modifies the test to choose a random memory limit and fixes
a bug that could cause windowFramer operators to skip a peer group
when seeking to frame start and end indexes in RANGE or GROUPS mode.

Release note: None

colexec: add range check for binary operations with timestamps

This commit adds a range check after additions or subtractions that
produce a timestamp result. This ensures that the vectorized engine
produces the same error as the row engine when the result of the
operation is out of range.

Release note: None

Fixes #37035

cockroach-teamcity · 2021-07-19T18:16:30Z

This change is

yuzefovich

Great work! This one is now quite easy given all the previous hard work. I only have a few minor comments.

One kind of test that I realized we're missing is making sure that all currently supported by the vectorized engine window functions are, in fact, being executed via the vectorized. I think originally, when I added a couple, I introduced some simple queries with row_number, rank, dense_rank, percent_rank, and cume_dist at the bottom of window logic test file, but it would probably be worth moving those into a separate file vectorize_window or something like that. There, we would use SET vectorize=experimental_always to make sure that the vectorized is used. And then extend the file with the recent increased support.

Reviewed 6 of 6 files at r1, 3 of 3 files at r2, 15 of 15 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)

pkg/sql/colexec/colexecwindow/first_last_nth_value_tmpl.go, line 155 at r3 (raw file):

			if partitionCol[i] {
				// Don't break for the start of the current partition.
				if !isPartitionStart || i != startIdx {

nit: I wonder whether it would be cleaner to pull this conditional outside of the loop and break here unconditionally. I'm thinking of something like:

i := startIdx
if isPartitionStart {
  i++
}
for ; i < n; i++ {
  if partitionCol[i] {
    break
  }
}

pkg/sql/colexec/execgen/cmd/execgen/first_last_nth_value_gen.go, line 37 at r3 (raw file):

}

func genFirstValueOp(inputFileContents string, wr io.Writer) error {

nit: these three generator functions are basically the same, and we could define a generator constructor in init() below that would be taking a couple of arguments and returning the correct generator. Take a look at projection_ops_gen.go for an example.

pkg/sql/colexec/execgen/cmd/execgen/overloads_bin.go, line 808 at r1 (raw file):

// timestampRangeCheck should be added at the end of operations that modify and
// return timestamps in order to ensure that the vectorized engine returns the
// same errors as the row engine.

nit: mention t_res local variable.

pkg/sql/colexec/execgen/cmd/execgen/overloads_bin.go, line 810 at r1 (raw file):

// same errors as the row engine.
const timestampRangeCheck = `	
if t_res.After(tree.MaxSupportedTime) || t_res.Before(tree.MinSupportedTime) {

Looks like the row engine also rounds to tree.Microsecond precision first, I think we should do too.

pkg/sql/colexec/execgen/cmd/execgen/range_offset_handler_gen.go, line 306 at r1 (raw file):

}

const dateToTimeCastStr = `

nit: add a quick comment about which "arguments" should this format string be used with.

yuzefovich

Oh, also, I'd be curious to see the absolute numbers of the microbenchmarks.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)

DrewKimball

Added a commit for the vectorized_window testfile.

Here are numbers for first_value, the other two are about the same:

cpu: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
BenchmarkWindowFunctions/FIRST_VALUE/rows=4096/partition=true/order=true-12         	    2079	    534864 ns/op
BenchmarkWindowFunctions/FIRST_VALUE/rows=4096/partition=true/order=false-12        	    2210	    537169 ns/op
BenchmarkWindowFunctions/FIRST_VALUE/rows=4096/partition=false/order=true-12        	    2211	    536847 ns/op
BenchmarkWindowFunctions/FIRST_VALUE/rows=4096/partition=false/order=false-12       	    2221	    534657 ns/op
BenchmarkWindowFunctions/FIRST_VALUE/rows=32768/partition=true/order=true-12        	     280	   4225680 ns/op
BenchmarkWindowFunctions/FIRST_VALUE/rows=32768/partition=true/order=false-12       	     284	   4223189 ns/op
BenchmarkWindowFunctions/FIRST_VALUE/rows=32768/partition=false/order=true-12       	     285	   4222705 ns/op
BenchmarkWindowFunctions/FIRST_VALUE/rows=32768/partition=false/order=false-12      	     283	   4244977 ns/op

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yuzefovich)

pkg/sql/colexec/colexecwindow/first_last_nth_value_tmpl.go, line 155 at r3 (raw file):