rowexec: use OutOfOrder mode of streamer for lookup joins with ordering

[yuzefovich]

Currently, the join reader always restores the required order for lookup
joins on its own since all looked up rows are buffered before any output
row is emitted. This observation allows us to use the OutOfOrder mode of
the streamer in such scenarios, so this commit makes such a change.
Previously, we would effectively maintain the order twice - both in the
streamer and in the join reader, and the former is redundant. This will
change in the future, but for now we can use the more-efficient mode.

name                                                  old time/op    new time/op    delta
LookupJoinEqColsAreKeyOrdering/Cockroach-24             6.64ms ± 1%    6.48ms ± 1%  -2.34%  (p=0.000 n=10+10)
LookupJoinEqColsAreKeyOrdering/MultinodeCockroach-24    7.89ms ± 1%    7.75ms ± 1%  -1.80%  (p=0.000 n=10+10)
LookupJoinOrdering/Cockroach-24                         9.01ms ± 3%    8.88ms ± 4%    ~     (p=0.218 n=10+10)
LookupJoinOrdering/MultinodeCockroach-24                12.1ms ± 4%    12.0ms ± 3%    ~     (p=0.393 n=10+10)

name                                                  old alloc/op   new alloc/op   delta
LookupJoinEqColsAreKeyOrdering/Cockroach-24             1.68MB ± 1%    1.60MB ± 1%  -4.93%  (p=0.000 n=10+10)
LookupJoinEqColsAreKeyOrdering/MultinodeCockroach-24    2.37MB ± 2%    2.29MB ± 2%  -3.11%  (p=0.000 n=10+10)
LookupJoinOrdering/Cockroach-24                         1.75MB ± 1%    1.66MB ± 1%  -5.01%  (p=0.000 n=10+9)
LookupJoinOrdering/MultinodeCockroach-24                2.36MB ± 1%    2.25MB ± 1%  -4.68%  (p=0.000 n=8+10)

name                                                  old allocs/op  new allocs/op  delta
LookupJoinEqColsAreKeyOrdering/Cockroach-24              10.0k ± 1%     10.0k ± 1%    ~     (p=0.278 n=10+9)
LookupJoinEqColsAreKeyOrdering/MultinodeCockroach-24     14.3k ± 1%     14.3k ± 1%    ~     (p=0.470 n=10+10)
LookupJoinOrdering/Cockroach-24                          12.4k ± 1%     12.5k ± 1%    ~     (p=0.780 n=10+10)
LookupJoinOrdering/MultinodeCockroach-24                 17.1k ± 1%     17.0k ± 1%    ~     (p=0.494 n=10+10)

Addresses: #82159.

Release note: None

[cockroach-teamcity]

This change is 

[DrewKimball]

Nice optimization!

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

---

pkg/kv/kvclient/kvstreamer/streamer_test.go line 509 at r1 (raw file):

	require.NoError(t, err)

	// The crux of the test - run a query that performs a lookup join when

This makes it sound like the point of this test (at least partially) was to test the InOrderMode of the streamer - are we already testing this well enough elsewhere? If not, a good way to test it might be to use an index join instead of a lookup join here.

[yuzefovich]

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

---

pkg/kv/kvclient/kvstreamer/streamer_test.go line 509 at r1 (raw file):

Previously, DrewKimball (Drew Kimball) wrote…

This makes it sound like the point of this test (at least partially) was to test the InOrderMode of the streamer - are we already testing this well enough elsewhere? If not, a good way to test it might be to use an index join instead of a lookup join here.

You're right that the true goal of this test is to exercise InOrder mode for ScanRequests that span multiple ranges (so that we test that the inOrderResultsBuffer correctly returns partial responses). We cannot use the index join for this because the index join returns exactly one row, and even if we have multiple column families, it is illegal to have a split point within a row (in other words, it is guaranteed that a single SQL row is always fully contained within a single KV range), so we have to use the lookup joins here. Typing this out made me realize that the changes to the test in this PR are meaningless, so I reverted them.

Still, we do have an explicit test TestInOrderResultsBuffer where we create scan responses with the randomized number of ranges, so the test coverage is still good, we just don't have an end-to-end test for this. I imagine that in the future we will start using the InOrder mode for lookup joins again, so I preserved the test.

[DrewKimball]

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

[yuzefovich]

TFTR!

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)