diff --git a/pkg/sql/delete.go b/pkg/sql/delete.go index 7879284b4769..42815ca7c38f 100644 --- a/pkg/sql/delete.go +++ b/pkg/sql/delete.go @@ -447,8 +447,9 @@ func canDeleteFast(ctx context.Context, source planNode, r *deleteRun) (*scanNod return nil, false } - // Check whether the source plan is "simple": that it contains no - // remaining filtering, limiting, sorting, etc. + // Check whether the source plan is "simple": that it contains no remaining + // filtering, limiting, sorting, etc. Note that this logic must be kept in + // sync with the logic for setting scanNode.isDeleteSource (see doExpandPlan.) // TODO(dt): We could probably be smarter when presented with an // index-join, but this goes away anyway once we push-down more of // SQL. diff --git a/pkg/sql/distsql_plan_stats.go b/pkg/sql/distsql_plan_stats.go index a70ed9d521af..939509787c33 100644 --- a/pkg/sql/distsql_plan_stats.go +++ b/pkg/sql/distsql_plan_stats.go @@ -40,7 +40,7 @@ func (dsp *DistSQLPlanner) createStatsPlan( if err != nil { return PhysicalPlan{}, err } - scan.spans, err = unconstrainedSpans(desc, scan.index) + scan.spans, err = unconstrainedSpans(desc, scan.index, scan.isDeleteSource) if err != nil { return PhysicalPlan{}, err } diff --git a/pkg/sql/distsqlplan/fake_span_resolver.go b/pkg/sql/distsqlplan/fake_span_resolver.go index 7540bd934237..ad125a637807 100644 --- a/pkg/sql/distsqlplan/fake_span_resolver.go +++ b/pkg/sql/distsqlplan/fake_span_resolver.go @@ -91,7 +91,7 @@ func (fit *fakeSpanResolverIterator) Seek( fit.err = err return } - if !splitKey.Equal(lastKey) { + if !splitKey.Equal(lastKey) && span.ContainsKey(splitKey) { splitKeys = append(splitKeys, splitKey) lastKey = splitKey } diff --git a/pkg/sql/expand_plan.go b/pkg/sql/expand_plan.go index f70b0d540f7d..fbb38522e1ec 100644 --- a/pkg/sql/expand_plan.go +++ b/pkg/sql/expand_plan.go @@ -121,6 +121,18 @@ func doExpandPlan( n.source, err = doExpandPlan(ctx, p, noParams, n.source) case *deleteNode: + // If the source of the delete is a scan node (optionally with a render on + // top), mark it as such. Note that this parallels the logic in + // canDeleteFast. + maybeScan := n.source + if sel, ok := maybeScan.(*renderNode); ok { + maybeScan = sel.source.plan + } + scan, ok := maybeScan.(*scanNode) + if ok { + scan.isDeleteSource = true + } + n.source, err = doExpandPlan(ctx, p, noParams, n.source) case *rowCountNode: diff --git a/pkg/sql/join_test.go b/pkg/sql/join_test.go index f9cdec364b69..b4fcfcbcecad 100644 --- a/pkg/sql/join_test.go +++ b/pkg/sql/join_test.go @@ -23,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/internal/client" "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/sql/opt/exec" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" @@ -41,7 +42,8 @@ func newTestScanNode(kvDB *client.DB, tableName string) (*scanNode, error) { return nil, err } scan.initOrdering(0 /* exactPrefix */, p.EvalContext()) - scan.spans, err = spansFromConstraint(desc, &desc.PrimaryIndex, nil /* constraint */) + scan.spans, err = spansFromConstraint( + desc, &desc.PrimaryIndex, nil /* constraint */, exec.ColumnOrdinalSet{}, false /* forDelete */) if err != nil { return nil, err } diff --git a/pkg/sql/logictest/testdata/logic_test/family b/pkg/sql/logictest/testdata/logic_test/family index 0cbb6c5d6337..a2f7814d53f3 100644 --- a/pkg/sql/logictest/testdata/logic_test/family +++ b/pkg/sql/logictest/testdata/logic_test/family @@ -37,6 +37,13 @@ SELECT * FROM abcd 1 2 3 4 5 6 7 8 +# Test point lookup, which triggers an optimization for only scanning one +# column family. +query I +SELECT c FROM abcd WHERE a = 1 +---- +3 + query I SELECT count(*) FROM abcd ---- @@ -81,6 +88,30 @@ SELECT * FROM abcd WHERE a = 1 ---- 1 NULL NULL NULL +# Test updating a NULL family +statement ok +INSERT INTO abcd (a) VALUES (2) + +query IIII +SELECT * FROM abcd WHERE a = 2 +---- +2 NULL NULL NULL + +statement ok +UPDATE abcd SET d = 5 WHERE a = 2 + +query IIII +SELECT * FROM abcd WHERE a = 2 +---- +2 NULL NULL 5 + +statement ok +DELETE FROM abcd WHERE a = 2 + +query IIII +SELECT * FROM abcd WHERE a = 2 +---- + statement ok ALTER TABLE abcd ADD e STRING FAMILY f1 diff --git a/pkg/sql/lookup_join.go b/pkg/sql/lookup_join.go index a7d733234b4a..2b5326919aa2 100644 --- a/pkg/sql/lookup_join.go +++ b/pkg/sql/lookup_join.go @@ -17,6 +17,7 @@ package sql import ( "context" + "github.com/cockroachdb/cockroach/pkg/sql/opt/exec" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" ) @@ -63,7 +64,9 @@ type lookupJoinRun struct { func (lj *lookupJoinNode) startExec(params runParams) error { // Make sure the table node has a span (full scan). var err error - lj.table.spans, err = spansFromConstraint(lj.table.desc, lj.table.index, nil /* constraint */) + lj.table.spans, err = spansFromConstraint( + lj.table.desc, lj.table.index, nil /* constraint */, exec.ColumnOrdinalSet{}, + false /* forDelete */) if err != nil { return err } diff --git a/pkg/sql/opt/exec/execbuilder/testdata/select_index b/pkg/sql/opt/exec/execbuilder/testdata/select_index index 0989c5d36dc8..4c54beba1227 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/select_index +++ b/pkg/sql/opt/exec/execbuilder/testdata/select_index @@ -1336,3 +1336,119 @@ render · · (w) · │ spans /1-/10 · · └── scan · · (v, w) · · table t3@primary · · + +# ------------------------------------------------------------------------------ +# These tests are for the point lookup optimization: for single row lookups on +# a table with multiple column families, we only scan the relevant column +# families. Note that this applies to SELECTs and UPDATEs but not DELETEs, since +# we need to ensure that we delete across all column families. +# ------------------------------------------------------------------------------ +statement ok +CREATE TABLE t4 ( + a INT, + b INT, + c INT, + d INT, + e INT, + PRIMARY KEY (a, b), + FAMILY (a, b), + FAMILY (c), + FAMILY (d), + FAMILY (e) +) + +statement ok +INSERT INTO t4 VALUES (10, 20, 30, 40, 50) + +# Point lookup on c does not touch the d or e families. +query TTT +EXPLAIN SELECT c FROM t4 WHERE a = 10 and b = 20 +---- +render · · + └── scan · · +· table t4@primary +· spans /10/20/0-/10/20/1/2 + +statement ok +SET tracing = on,kv,results; SELECT c FROM t4 WHERE a = 10 and b = 20; SET tracing = off + +query T +SELECT message FROM [SHOW KV TRACE FOR SESSION] + WHERE message LIKE 'fetched:%' OR message LIKE 'output row%' +---- +fetched: /t4/primary/10/20 -> NULL +fetched: /t4/primary/10/20/c -> 30 +output row: [30] + +# Point lookup on d does not touch the c or e families. +query TTT +EXPLAIN SELECT d FROM t4 WHERE a = 10 and b = 20 +---- +render · · + └── scan · · +· table t4@primary +· spans /10/20/0-/10/20/1 /10/20/2/1-/10/20/2/2 + +statement ok +SET tracing = on,kv,results; SELECT d FROM t4 WHERE a = 10 and b = 20; SET tracing = off + +query T +SELECT message FROM [SHOW KV TRACE FOR SESSION] + WHERE message LIKE 'fetched:%' OR message LIKE 'output row%' +---- +fetched: /t4/primary/10/20 -> NULL +fetched: /t4/primary/10/20/c -> 40 +output row: [40] + +# Point lookup on both d and e uses a single span for the two adjacent column +# families. +query TTT +EXPLAIN SELECT d, e FROM t4 WHERE a = 10 and b = 20 +---- +render · · + └── scan · · +· table t4@primary +· spans /10/20/0-/10/20/1 /10/20/2/1-/10/20/3/2 + +# Optimization should also be applied for updates. +query TTT +EXPLAIN UPDATE t4 SET c = 30 WHERE a = 10 and b = 20 +---- +count · · + └── update · · + │ table t4 + │ set c + └── render · · + └── scan · · +· table t4@primary +· spans /10/20/0-/10/20/1/2 + +# Optimization should not be applied for deletes. +query TTT +EXPLAIN DELETE FROM t4 WHERE a = 10 and b = 20 +---- +count · · + └── delete · · + │ from t4 + └── render · · + └── scan · · +· table t4@primary +· spans /10/20-/10/20/# + +# Optimization should not be applied for non point lookups. +query TTT +EXPLAIN SELECT c FROM t4 WHERE a = 10 and b >= 20 and b < 22 +---- +render · · + └── scan · · +· table t4@primary +· spans /10/20-/10/21/# + +# Optimization should not be applied for partial primary key filter. +query TTT +EXPLAIN SELECT c FROM t4 WHERE a = 10 +---- +render · · + └── scan · · +· table t4@primary +· spans /10-/11 diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index a55039c6ec2d..b1ad23d945f5 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -101,7 +101,8 @@ func (ef *execFactory) ConstructScan( scan.hardLimit = hardLimit scan.reverse = reverse var err error - scan.spans, err = spansFromConstraint(tabDesc, indexDesc, indexConstraint) + scan.spans, err = spansFromConstraint( + tabDesc, indexDesc, indexConstraint, cols, scan.isDeleteSource) if err != nil { return nil, err } diff --git a/pkg/sql/opt_index_selection.go b/pkg/sql/opt_index_selection.go index d4bbeba41292..593348312461 100644 --- a/pkg/sql/opt_index_selection.go +++ b/pkg/sql/opt_index_selection.go @@ -21,9 +21,11 @@ import ( "github.com/pkg/errors" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/opt" "github.com/cockroachdb/cockroach/pkg/sql/opt/constraint" + "github.com/cockroachdb/cockroach/pkg/sql/opt/exec" "github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder" "github.com/cockroachdb/cockroach/pkg/sql/opt/idxconstraint" "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" @@ -99,7 +101,7 @@ func (p *planner) selectIndex( // No where-clause, no ordering, and no specified index. s.initOrdering(0 /* exactPrefix */, p.EvalContext()) var err error - s.spans, err = unconstrainedSpans(s.desc, s.index) + s.spans, err = unconstrainedSpans(s.desc, s.index, s.isDeleteSource) if err != nil { return nil, errors.Wrapf(err, "table ID = %d, index ID = %d", s.desc.ID, s.index.ID) } @@ -229,7 +231,8 @@ func (p *planner) selectIndex( s.run.isSecondaryIndex = (c.index != &s.desc.PrimaryIndex) var err error - s.spans, err = spansFromConstraint(s.desc, c.index, c.ic.Constraint()) + s.spans, err = spansFromConstraint( + s.desc, c.index, c.ic.Constraint(), s.valNeededForCol, s.isDeleteSource) if err != nil { return nil, errors.Wrapf( err, "constraint = %s, table ID = %d, index ID = %d", @@ -493,9 +496,9 @@ func (v *indexInfo) makeIndexConstraints( } func unconstrainedSpans( - tableDesc *sqlbase.TableDescriptor, index *sqlbase.IndexDescriptor, + tableDesc *sqlbase.TableDescriptor, index *sqlbase.IndexDescriptor, forDelete bool, ) (roachpb.Spans, error) { - return spansFromConstraint(tableDesc, index, nil) + return spansFromConstraint(tableDesc, index, nil, exec.ColumnOrdinalSet{}, forDelete) } // spansFromConstraint converts the spans in a Constraint to roachpb.Spans. @@ -503,7 +506,11 @@ func unconstrainedSpans( // interstices are pieces of the key that need to be inserted after each column // (for interleavings). func spansFromConstraint( - tableDesc *sqlbase.TableDescriptor, index *sqlbase.IndexDescriptor, c *constraint.Constraint, + tableDesc *sqlbase.TableDescriptor, + index *sqlbase.IndexDescriptor, + c *constraint.Constraint, + needed exec.ColumnOrdinalSet, + forDelete bool, ) (roachpb.Spans, error) { interstices := make([][]byte, len(index.ColumnDirections)+len(index.ExtraColumnIDs)+1) interstices[0] = sqlbase.MakeIndexKeyPrefix(tableDesc, index.ID) @@ -527,22 +534,25 @@ func spansFromConstraint( encoding.EncodeUvarintAscending(interstices[sharedPrefixLen], uint64(index.ID)) } + var spans roachpb.Spans + var err error if c == nil || c.IsUnconstrained() { // Encode a full span. - sp, err := spanFromConstraintSpan(tableDesc, index, &constraint.UnconstrainedSpan, interstices) + spans, err = appendSpansFromConstraintSpan( + spans, tableDesc, index, &constraint.UnconstrainedSpan, interstices, needed, forDelete) if err != nil { return nil, err } - return roachpb.Spans{sp}, nil + return spans, nil } - spans := make(roachpb.Spans, c.Spans.Count()) - for i := range spans { - s, err := spanFromConstraintSpan(tableDesc, index, c.Spans.Get(i), interstices) + spans = make(roachpb.Spans, 0, c.Spans.Count()) + for i := 0; i < c.Spans.Count(); i++ { + spans, err = appendSpansFromConstraintSpan( + spans, tableDesc, index, c.Spans.Get(i), interstices, needed, forDelete) if err != nil { return nil, err } - spans[i] = s } return spans, nil } @@ -590,19 +600,26 @@ func encodeConstraintKey( return key, nil } -// spanFromConstraintSpan converts a constraint.Span to a roachpb.Span. -func spanFromConstraintSpan( +// appendSpansFromConstraintSpan converts a constraint.Span to one or more +// roachpb.Spans and appends them to the provided spans. It appends multiple +// spans in the case that multiple, non-adjacent column families should be +// scanned. The forDelete parameter indicates whether these spans will be used +// for row deletion. +func appendSpansFromConstraintSpan( + spans roachpb.Spans, tableDesc *sqlbase.TableDescriptor, index *sqlbase.IndexDescriptor, cs *constraint.Span, interstices [][]byte, -) (roachpb.Span, error) { + needed exec.ColumnOrdinalSet, + forDelete bool, +) (roachpb.Spans, error) { var s roachpb.Span var err error // Encode each logical part of the start key. s.Key, err = encodeConstraintKey(index, cs.StartKey(), interstices) if err != nil { - return roachpb.Span{}, err + return nil, err } if cs.StartBoundary() == constraint.IncludeBoundary { s.Key = append(s.Key, interstices[cs.StartKey().Length()]...) @@ -613,18 +630,70 @@ func spanFromConstraintSpan( // Encode each logical part of the end key. s.EndKey, err = encodeConstraintKey(index, cs.EndKey(), interstices) if err != nil { - return roachpb.Span{}, err + return nil, err } s.EndKey = append(s.EndKey, interstices[cs.EndKey().Length()]...) + // Optimization: for single row lookups on a table with multiple column + // families, only scan the relevant column families. This is disabled for + // deletions to ensure that the entire row is deleted. + if !forDelete && + needed.Len() > 0 && + index.ID == tableDesc.PrimaryIndex.ID && + len(tableDesc.Families) > 1 && + cs.StartKey().Length() == len(tableDesc.PrimaryIndex.ColumnIDs) && + s.Key.Equal(s.EndKey) { + neededFamilyIDs := neededColumnFamilyIDs(tableDesc, needed) + if len(neededFamilyIDs) < len(tableDesc.Families) { + for i, familyID := range neededFamilyIDs { + var span roachpb.Span + span.Key = make(roachpb.Key, len(s.Key)) + copy(span.Key, s.Key) + span.Key = keys.MakeFamilyKey(span.Key, uint32(familyID)) + span.EndKey = span.Key.PrefixEnd() + if i > 0 && familyID == neededFamilyIDs[i-1]+1 { + // This column family is adjacent to the previous one. We can merge + // the two spans into one. + spans[len(spans)-1].EndKey = span.EndKey + } else { + spans = append(spans, span) + } + } + return spans, nil + } + } + // We tighten the end key to prevent reading interleaved children after the // last parent key. If cs.End.Inclusive is true, we also advance the key as // necessary. endInclusive := cs.EndBoundary() == constraint.IncludeBoundary s.EndKey, err = sqlbase.AdjustEndKeyForInterleave(tableDesc, index, s.EndKey, endInclusive) if err != nil { - return roachpb.Span{}, err + return nil, err } + return append(spans, s), nil +} + +func neededColumnFamilyIDs( + tableDesc *sqlbase.TableDescriptor, neededCols exec.ColumnOrdinalSet, +) []sqlbase.FamilyID { + colIdxMap := tableDesc.ColumnIdxMap() + + var needed []sqlbase.FamilyID + for _, family := range tableDesc.Families { + for _, columnID := range family.ColumnIDs { + columnOrdinal := colIdxMap[columnID] + if neededCols.Contains(columnOrdinal) { + needed = append(needed, family.ID) + break + } + } + } + + // TODO(solon): There is a further optimization possible here: if there is at + // least one non-nullable column in the needed column families, we can + // potentially omit the primary family, since the primary keys are encoded + // in all families. (Note that composite datums are an exception.) - return s, nil + return needed } diff --git a/pkg/sql/opt_index_selection_test.go b/pkg/sql/opt_index_selection_test.go index 63664e6f4601..abce820349ea 100644 --- a/pkg/sql/opt_index_selection_test.go +++ b/pkg/sql/opt_index_selection_test.go @@ -24,6 +24,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/opt/constraint" + "github.com/cockroachdb/cockroach/pkg/sql/opt/exec" "github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder" "github.com/cockroachdb/cockroach/pkg/sql/opt/xform" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -110,7 +111,8 @@ func makeSpans( t.Fatal(err) } - spans, err = spansFromConstraint(desc, index, c.ic.Constraint()) + spans, err = spansFromConstraint(desc, index, c.ic.Constraint(), exec.ColumnOrdinalSet{}, + false /* forDelete */) if err != nil { t.Fatal(err) } diff --git a/pkg/sql/scan.go b/pkg/sql/scan.go index 2957cd51842f..2ded3b55476b 100644 --- a/pkg/sql/scan.go +++ b/pkg/sql/scan.go @@ -109,6 +109,9 @@ type scanNode struct { // Set when the scanNode is crated via the exec factory. createdByOpt bool + + // Indicates if this scan is the source for a delete node. + isDeleteSource bool } // scanVisibility represents which table columns should be included in a scan.