Skip to content

Commit

Permalink
distsql: don't use sortTopK when filter is present
Browse files Browse the repository at this point in the history
The sorter was producing incorrect results when both a limit and a
filter were applied. We can't use sortTopK in this case since some
results may be filtered out in post-processing. Note this scenario is
somewhat rare because typically the filter would be pushed down below
the sort. The issue was observed when selecting from the result of a
SHOW TRACE.

Also removed a bit of dead code.

Fixes #31163

Release note: None
  • Loading branch information
solongordon committed Oct 10, 2018
1 parent 30d91b6 commit cf5020d
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 13 deletions.
14 changes: 1 addition & 13 deletions pkg/sql/distsqlrun/sorter.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ type sorterBase struct {
input RowSource
ordering sqlbase.ColumnOrdering
matchLen uint32
// count is the maximum number of rows that the sorter will push to the
// ProcOutputHelper. 0 if the sorter should sort and push all the rows from
// the input.
count int64

rows sortableRowContainer
i rowIterator
Expand All @@ -56,13 +52,6 @@ func (s *sorterBase) init(
matchLen uint32,
opts ProcStateOpts,
) error {
count := int64(0)
if post.Limit != 0 {
// The sorter needs to produce Offset + Limit rows. The ProcOutputHelper
// will discard the first Offset ones.
count = int64(post.Limit) + int64(post.Offset)
}

ctx := flowCtx.EvalCtx.Ctx()
if sp := opentracing.SpanFromContext(ctx); sp != nil && tracing.IsRecording(sp) {
input = NewInputStatCollector(input)
Expand Down Expand Up @@ -116,7 +105,6 @@ func (s *sorterBase) init(
s.input = input
s.ordering = ordering
s.matchLen = matchLen
s.count = count
return nil
}

Expand Down Expand Up @@ -209,7 +197,7 @@ func newSorter(
output RowReceiver,
) (Processor, error) {
count := int64(0)
if post.Limit != 0 {
if post.Limit != 0 && post.Filter.Expr == "" {
// The sorter needs to produce Offset + Limit rows. The ProcOutputHelper
// will discard the first Offset ones.
count = int64(post.Limit) + int64(post.Offset)
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/limit
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,12 @@ SELECT k, v FROM t ORDER BY k LIMIT (SELECT count(*)-3 FROM t) OFFSET (SELECT co
2 -4
3 9
4 -16

# Test sort node with both filter and limit. (https://github.com/cockroachdb/cockroach/issues/31163)
statement ok
SET TRACING = ON; SELECT 1; SET TRACING = OFF

query I
SELECT SPAN FROM [SHOW TRACE FOR SESSION] WHERE span = 1 LIMIT 1
----
1

0 comments on commit cf5020d

Please sign in to comment.