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.

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

sql: avoid copying ColumnDescriptors in initColsForScan #50727

Merged
merged 1 commit into from
Jun 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,7 @@ func tableOrdinal(
// toTableOrdinals returns a mapping from column ordinals in cols to table
// reader column ordinals.
func toTableOrdinals(
cols []sqlbase.ColumnDescriptor,
cols []*sqlbase.ColumnDescriptor,
desc *sqlbase.ImmutableTableDescriptor,
visibility execinfrapb.ScanVisibility,
) []int {
Expand All @@ -934,17 +934,17 @@ func toTableOrdinals(
// getOutputColumnsFromColsForScan returns the indices of the columns that are
// returned by a scanNode or a tableReader.
// If remap is not nil, the column ordinals are remapped accordingly.
func getOutputColumnsFromColsForScan(cols []sqlbase.ColumnDescriptor, remap []int) []uint32 {
outputColumns := make([]uint32, 0, len(cols))
func getOutputColumnsFromColsForScan(cols []*sqlbase.ColumnDescriptor, remap []int) []uint32 {
outputColumns := make([]uint32, len(cols))
// TODO(radu): if we have a scan with a filter, cols will include the
// columns needed for the filter, even if they aren't needed for the next
// stage.
for i := 0; i < len(cols); i++ {
for i := range outputColumns {
colIdx := i
if remap != nil {
colIdx = remap[i]
}
outputColumns = append(outputColumns, uint32(colIdx))
outputColumns[i] = uint32(colIdx)
}
return outputColumns
}
Expand Down Expand Up @@ -1085,7 +1085,7 @@ type tableReaderPlanningInfo struct {
maxResults uint64
estimatedRowCount uint64
reqOrdering ReqOrdering
cols []sqlbase.ColumnDescriptor
cols []*sqlbase.ColumnDescriptor
colsToTableOrdrinalMap []int
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/distsql_spec_exec_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (e *distSQLSpecExecFactory) ConstructScan(
if err != nil {
return nil, err
}
p.ResultColumns = sqlbase.ResultColumnsFromColDescs(tabDesc.GetID(), cols)
p.ResultColumns = sqlbase.ResultColumnsFromColDescPtrs(tabDesc.GetID(), cols)

if indexConstraint != nil && indexConstraint.IsContradiction() {
// TODO(yuzefovich): once ConstructValues is implemented, consider
Expand Down
15 changes: 9 additions & 6 deletions pkg/sql/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type scanNode struct {
// be gained (e.g. for tables with wide rows) by reading only certain
// columns from KV using point lookups instead of a single range lookup for
// the entire row.
cols []sqlbase.ColumnDescriptor
cols []*sqlbase.ColumnDescriptor
// There is a 1-1 correspondence between cols and resultColumns.
resultColumns sqlbase.ResultColumns

Expand Down Expand Up @@ -280,12 +280,12 @@ func (n *scanNode) lookupSpecifiedIndex(indexFlags *tree.IndexFlags) error {
// initColsForScan initializes cols according to desc and colCfg.
func initColsForScan(
desc *sqlbase.ImmutableTableDescriptor, colCfg scanColumnsConfig,
) (cols []sqlbase.ColumnDescriptor, err error) {
) (cols []*sqlbase.ColumnDescriptor, err error) {
if colCfg.wantedColumns == nil {
return nil, errors.AssertionFailedf("unexpectedly wantedColumns is nil")
}

cols = make([]sqlbase.ColumnDescriptor, 0, len(desc.ReadableColumns))
cols = make([]*sqlbase.ColumnDescriptor, 0, len(desc.ReadableColumns))
for _, wc := range colCfg.wantedColumns {
var c *sqlbase.ColumnDescriptor
var err error
Expand All @@ -298,7 +298,7 @@ func initColsForScan(
return cols, err
}

cols = append(cols, *c)
cols = append(cols, c)
}

if colCfg.addUnwantedAsHidden {
Expand All @@ -312,9 +312,12 @@ func initColsForScan(
}
}
if !found {
// NB: we could amortize this allocation using a second slice,
// but addUnwantedAsHidden is only used by scrub, so doing so
// doesn't seem worth it.
col := *c
col.Hidden = true
cols = append(cols, col)
cols = append(cols, &col)
}
}
}
Expand All @@ -334,7 +337,7 @@ func (n *scanNode) initDescDefaults(colCfg scanColumnsConfig) error {
}

// Set up the rest of the scanNode.
n.resultColumns = sqlbase.ResultColumnsFromColDescs(n.desc.GetID(), n.cols)
n.resultColumns = sqlbase.ResultColumnsFromColDescPtrs(n.desc.GetID(), n.cols)
n.colIdxMap = make(map[sqlbase.ColumnID]int, len(n.cols))
for i, c := range n.cols {
n.colIdxMap[c.ID] = i
Expand Down
43 changes: 26 additions & 17 deletions pkg/sql/sqlbase/result_columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,37 @@ type ResultColumn struct {
// describe the column types of a table.
type ResultColumns []ResultColumn

// ResultColumnsFromColDescs converts ColumnDescriptors to ResultColumns.
// ResultColumnsFromColDescs converts []ColumnDescriptor to []ResultColumn.
func ResultColumnsFromColDescs(tableID ID, colDescs []ColumnDescriptor) ResultColumns {
cols := make(ResultColumns, 0, len(colDescs))
for i := range colDescs {
// Convert the ColumnDescriptor to ResultColumn.
colDesc := &colDescs[i]
return resultColumnsFromColDescs(tableID, len(colDescs), func(i int) *ColumnDescriptor {
return &colDescs[i]
})
}

// ResultColumnsFromColDescPtrs converts []*ColumnDescriptor to []ResultColumn.
func ResultColumnsFromColDescPtrs(tableID ID, colDescs []*ColumnDescriptor) ResultColumns {
return resultColumnsFromColDescs(tableID, len(colDescs), func(i int) *ColumnDescriptor {
return colDescs[i]
})
}

func resultColumnsFromColDescs(
tableID ID, numCols int, getColDesc func(int) *ColumnDescriptor,
) ResultColumns {
cols := make(ResultColumns, numCols)
for i := range cols {
colDesc := getColDesc(i)
typ := colDesc.Type
if typ == nil {
panic(fmt.Sprintf("unsupported column type: %s", colDesc.Type.Family()))
}

hidden := colDesc.Hidden
cols = append(
cols,
ResultColumn{
Name: colDesc.Name,
Typ: typ,
Hidden: hidden,
TableID: tableID,
PGAttributeNum: colDesc.GetLogicalColumnID(),
},
)
cols[i] = ResultColumn{
Name: colDesc.Name,
Typ: typ,
Hidden: colDesc.Hidden,
TableID: tableID,
PGAttributeNum: colDesc.GetLogicalColumnID(),
}
}
return cols
}
Expand Down