Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

distsql: copy prefix key in index skip table reader #39636

Merged
merged 1 commit into from
Aug 14, 2019
Merged

distsql: copy prefix key in index skip table reader #39636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions pkg/sql/distsqlrun/index_skip_table_reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ type indexSkipTableReader struct {
// currentSpan maintains which span we are currently scanning.
currentSpan int

// scratchKey is scratch space to build prefix keys for following iterations.
scratchKey []byte

// keyPrefixLen holds the length of the prefix of the index
// that we are performing a distinct over.
keyPrefixLen int
Expand Down Expand Up @@ -108,6 +111,7 @@ func newIndexSkipTableReader(
return nil, err
}
t.indexLen = len(index.ColumnIDs)
t.scratchKey = make([]byte, 0, t.indexLen)

cols := immutDesc.Columns
if returnMutations {
Expand Down Expand Up @@ -181,10 +185,17 @@ func (t *indexSkipTableReader) Next() (sqlbase.EncDatumRow, *distsqlpb.ProducerM
}
}

key, err := t.fetcher.PartialKey(t.keyPrefixLen)
if err != nil {
t.MoveToDraining(err)
return nil, &distsqlpb.ProducerMetadata{Err: err}
{
// Fetch the PartialKey in a separate scope so that it cannot be reused
// outside this scope.
key, err := t.fetcher.PartialKey(t.keyPrefixLen)
if err != nil {
t.MoveToDraining(err)
return nil, &distsqlpb.ProducerMetadata{Err: err}
}
// Modifying key in place would corrupt our current row, so make a copy.
// Use an append here to ensure we copy all elements.
t.scratchKey = append(t.scratchKey[:0], key...)
}

row, _, _, err := t.fetcher.NextRow(t.Ctx)
Expand All @@ -203,16 +214,21 @@ func (t *indexSkipTableReader) Next() (sqlbase.EncDatumRow, *distsqlpb.ProducerM
// our new key is larger than any value with the same prefix, we place
// 0xff at all other index column values, and one more to guard against
// 0xff present as a value in the table (0xff encodes a type of null).
// TODO(asubiotto): We should delegate to PrefixEnd here (it exists for
// this purpose and is widely used), but we still need to handle maximal
// keys. Maybe we should just exit early (t.currentSpan++) when we
// encounter one. We also need a test to ensure that we behave properly
// when we encounter maximal (i.e. all nulls) prefix keys.
for i := 0; i < (t.indexLen - t.keyPrefixLen + 1); i++ {
key = append(key, 0xff)
t.scratchKey = append(t.scratchKey, 0xff)
}
t.spans[t.currentSpan].Key = key
t.spans[t.currentSpan].Key = t.scratchKey
} else {
// In the case of reverse, this is much easier. The reverse fetcher
// returns the key retrieved, in this case the first key smaller
// than EndKey in the current span. Since EndKey is exclusive, we
// just set the retrieved key as EndKey for the next scan.
t.spans[t.currentSpan].EndKey = key
t.spans[t.currentSpan].EndKey = t.scratchKey
}

// If the changes we made turned our current span invalid, mark that
Expand Down
85 changes: 30 additions & 55 deletions pkg/sql/distsqlrun/index_skip_table_reader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected string
}{
{
desc: "Distinct scan simple",
// Distinct scan simple.
desc: "SimpleForward",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td1.PrimaryIndexSpan()}},
Expand All @@ -177,7 +178,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[0] [1] [2] [3] [4] [5] [6] [7] [8] [9]]",
},
{
desc: "Distinct scan on interleaved table parent",
// Distinct scan on interleaved table parent.
desc: "InterleavedParent",
tableDesc: td5,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td5.PrimaryIndexSpan()}},
Expand All @@ -189,7 +191,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[0] [1] [2] [3] [4] [5] [6] [7] [8] [9]]",
},
{
desc: "Distinct scan on interleaved table child",
// Distinct scan on interleaved table child.
desc: "InterleavedChild",
tableDesc: td6,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td6.PrimaryIndexSpan()}},
Expand All @@ -201,7 +204,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[1] [2] [3] [4] [5] [6] [7] [8] [9] [10]]",
},
{
desc: "Distinct scan with multiple spans",
// Distinct scan with multiple spans.
desc: "MultipleSpans",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{makeIndexSpan(td1, 0, 3), makeIndexSpan(td1, 5, 8)},
Expand All @@ -213,7 +217,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[0] [1] [2] [5] [6] [7]]",
},
{
desc: "Distinct scan with multiple spans and filter",
// Distinct scan with multiple spans and filter,
desc: "MultipleSpansWithFilter",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{makeIndexSpan(td1, 0, 3), makeIndexSpan(td1, 5, 8)},
Expand All @@ -226,7 +231,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[5] [6]]",
},
{
desc: "Distinct scan with filter",
// Distinct scan with filter.
desc: "Filter",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td1.PrimaryIndexSpan()}},
Expand All @@ -239,7 +245,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[4] [5] [6]]",
},
{
desc: "Distinct scan with multiple requested columns",
// Distinct scan with multiple requested columns.
desc: "MultipleOutputCols",
tableDesc: td2,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td2.PrimaryIndexSpan()}},
Expand All @@ -251,7 +258,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[0 1] [1 2] [2 3] [3 4]]",
},
{
desc: "Distinct scan on table with NULLs",
// Distinct scan on table with NULLs.
desc: "Nulls",
tableDesc: td3,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td3.PrimaryIndexSpan()}},
Expand All @@ -263,7 +271,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[0] [1] [2] [3]]",
},
{
desc: "Distinct scan on secondary index",
// Distinct scan on secondary index",
desc: "SecondaryIdx",
tableDesc: td4,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td4.IndexSpan(2)}},
Expand All @@ -276,7 +285,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[1] [2] [3] [4] [5] [6] [7] [8] [9] [10]]",
},
{
desc: "Distinct reverse scan simple",
// Distinct reverse scan simple.
desc: "SimpleReverse",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td1.PrimaryIndexSpan()}},
Expand All @@ -289,7 +299,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[9] [8] [7] [6] [5] [4] [3] [2] [1] [0]]",
},
{
desc: "Distinct reverse scan with multiple spans",
// Distinct reverse scan with multiple spans.
desc: "MultipleSpansReverse",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{makeIndexSpan(td1, 0, 3), makeIndexSpan(td1, 5, 8)},
Expand All @@ -302,7 +313,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[7] [6] [5] [2] [1] [0]]",
},
{
desc: "Distinct reverse scan with multiple spans and filter",
// Distinct reverse scan with multiple spans and filter.
desc: "MultipleSpansWithFilterReverse",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{makeIndexSpan(td1, 0, 3), makeIndexSpan(td1, 5, 8)},
Expand All @@ -316,47 +328,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[6] [5]]",
},
{
desc: "Distinct reverse scan simple",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td1.PrimaryIndexSpan()}},
Reverse: true,
},
post: distsqlpb.PostProcessSpec{
Projection: true,
OutputColumns: []uint32{0},
},
expected: "[[9] [8] [7] [6] [5] [4] [3] [2] [1] [0]]",
},
{
desc: "Distinct reverse scan with multiple spans",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{makeIndexSpan(td1, 0, 3), makeIndexSpan(td1, 5, 8)},
Reverse: true,
},
post: distsqlpb.PostProcessSpec{
Projection: true,
OutputColumns: []uint32{0},
},
expected: "[[7] [6] [5] [2] [1] [0]]",
},
{
desc: "Distinct reverse scan with multiple spans and filter",
tableDesc: td1,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{makeIndexSpan(td1, 0, 3), makeIndexSpan(td1, 5, 8)},
Reverse: true,
},
post: distsqlpb.PostProcessSpec{
Filter: distsqlpb.Expression{Expr: "@1 > 3 AND @1 < 7"},
Projection: true,
OutputColumns: []uint32{0},
},
expected: "[[6] [5]]",
},
{
desc: "Distinct reverse scan on interleaved parent",
// Distinct reverse scan on interleaved parent.
desc: "InterleavedParentReverse",
tableDesc: td5,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td5.PrimaryIndexSpan()}},
Expand All @@ -369,7 +342,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[9] [8] [7] [6] [5] [4] [3] [2] [1] [0]]",
},
{
desc: "Distinct reverse scan with multiple spans on interleaved parent",
// Distinct reverse scan with multiple spans on interleaved parent.
desc: "InterleavedParentMultipleSpansReverse",
tableDesc: td5,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{makeIndexSpan(td5, 0, 3), makeIndexSpan(td5, 5, 8)},
Expand All @@ -382,7 +356,8 @@ func TestIndexSkipTableReader(t *testing.T) {
expected: "[[7] [6] [5] [2] [1] [0]]",
},
{
desc: "Distinct reverse scan on interleaved child",
// Distinct reverse scan on interleaved child.
desc: "InterleavedChildReverse",
tableDesc: td6,
spec: distsqlpb.IndexSkipTableReaderSpec{
Spans: []distsqlpb.TableReaderSpan{{Span: td6.PrimaryIndexSpan()}},
Expand Down