Skip to content

Commit

Permalink
sql: remove separate scanVisilibity struct
Browse files Browse the repository at this point in the history
This commit removes `sql.scanVisibility` in favor of protobuf-generated
`execinfrapb.ScanVisibility`. It also introduces prettier aliases for
the two values into `execinfra` package that are now used throughout the
code.

Release note: None
  • Loading branch information
yuzefovich committed Jun 2, 2020
1 parent a7b1c3c commit 6ecbc02
Show file tree
Hide file tree
Showing 17 changed files with 90 additions and 101 deletions.
4 changes: 2 additions & 2 deletions pkg/sql/colexec/colbatch_scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func newColBatchScan(

limitHint := execinfra.LimitHint(spec.LimitHint, post)

returnMutations := spec.Visibility == execinfrapb.ScanVisibility_PUBLIC_AND_NOT_PUBLIC
returnMutations := spec.Visibility == execinfra.ScanVisibilityPublicAndNotPublic
typs := spec.Table.ColumnTypesWithMutations(returnMutations)
evalCtx := flowCtx.NewEvalCtx()
// Before we can safely use types from the table descriptor, we need to
Expand Down Expand Up @@ -182,7 +182,7 @@ func initCRowFetcher(
}

cols := immutDesc.Columns
if scanVisibility == execinfrapb.ScanVisibility_PUBLIC_AND_NOT_PUBLIC {
if scanVisibility == execinfra.ScanVisibilityPublicAndNotPublic {
cols = immutDesc.ReadableColumns
}
tableArgs := row.FetcherTableArgs{
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/colexec/execplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ func NewColOperator(
// still responsible for doing the cancellation check on their own while
// performing long operations.
result.Op = NewCancelChecker(result.Op)
returnMutations := core.TableReader.Visibility == execinfrapb.ScanVisibility_PUBLIC_AND_NOT_PUBLIC
returnMutations := core.TableReader.Visibility == execinfra.ScanVisibilityPublicAndNotPublic
result.ColumnTypes = core.TableReader.Table.ColumnTypesWithMutations(returnMutations)
case core.Aggregator != nil:
if err := checkNumIn(inputs, 1); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ func (d *deleteNode) processSourceRow(params runParams, sourceVals tree.Datums)
// If result rows need to be accumulated, do it.
if d.run.rows != nil {
// The new values can include all columns, the construction of the
// values has used publicAndNonPublicColumns so the values may
// contain additional columns for every newly dropped column not
// visible. We do not want them to be available for RETURNING.
// values has used execinfra.ScanVisibilityPublicAndNotPublic so the
// values may contain additional columns for every newly dropped column
// not visible. We do not want them to be available for RETURNING.
//
// d.run.rows.NumCols() is guaranteed to only contain the requested
// public columns.
Expand Down
14 changes: 8 additions & 6 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ func initTableReaderSpec(
Table: *n.desc.TableDesc(),
Reverse: n.reverse,
IsCheck: n.isCheck,
Visibility: n.colCfg.visibility.toDistSQLScanVisibility(),
Visibility: n.colCfg.visibility,
LockingStrength: n.lockingStrength,
LockingWaitPolicy: n.lockingWaitPolicy,

Expand Down Expand Up @@ -926,14 +926,16 @@ func initTableReaderSpec(

// scanNodeOrdinal returns the index of a column with the given ID.
func tableOrdinal(
desc *sqlbase.ImmutableTableDescriptor, colID sqlbase.ColumnID, visibility scanVisibility,
desc *sqlbase.ImmutableTableDescriptor,
colID sqlbase.ColumnID,
visibility execinfrapb.ScanVisibility,
) int {
for i := range desc.Columns {
if desc.Columns[i].ID == colID {
return i
}
}
if visibility == publicAndNonPublicColumns {
if visibility == execinfra.ScanVisibilityPublicAndNotPublic {
offset := len(desc.Columns)
for i, col := range desc.MutationColumns() {
if col.ID == colID {
Expand Down Expand Up @@ -1107,7 +1109,7 @@ func (dsp *DistSQLPlanner) createTableReaders(
p.ResultRouters = make([]physicalplan.ProcessorIdx, len(spanPartitions))
p.Processors = make([]physicalplan.Processor, 0, len(spanPartitions))

returnMutations := n.colCfg.visibility == publicAndNonPublicColumns
returnMutations := n.colCfg.visibility == execinfra.ScanVisibilityPublicAndNotPublic

for i, sp := range spanPartitions {
var tr *execinfrapb.TableReaderSpec
Expand Down Expand Up @@ -1833,7 +1835,7 @@ func (dsp *DistSQLPlanner) createPlanForIndexJoin(
joinReaderSpec := execinfrapb.JoinReaderSpec{
Table: *n.table.desc.TableDesc(),
IndexIdx: 0,
Visibility: n.table.colCfg.visibility.toDistSQLScanVisibility(),
Visibility: n.table.colCfg.visibility,
LockingStrength: n.table.lockingStrength,
LockingWaitPolicy: n.table.lockingWaitPolicy,
}
Expand Down Expand Up @@ -1900,7 +1902,7 @@ func (dsp *DistSQLPlanner) createPlanForLookupJoin(
joinReaderSpec := execinfrapb.JoinReaderSpec{
Table: *n.table.desc.TableDesc(),
Type: n.joinType,
Visibility: n.table.colCfg.visibility.toDistSQLScanVisibility(),
Visibility: n.table.colCfg.visibility,
LockingStrength: n.table.lockingStrength,
LockingWaitPolicy: n.table.lockingWaitPolicy,
MaintainOrdering: len(n.reqOrdering) > 0,
Expand Down
11 changes: 10 additions & 1 deletion pkg/sql/execinfra/scanbase.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@

package execinfra

import "github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
import (
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
)

// ParallelScanResultThreshold is the number of results up to which, if the
// maximum number of results returned by a scan is known, the table reader
Expand All @@ -32,3 +35,9 @@ func ScanShouldLimitBatches(maxResults uint64, limitHint int64, flowCtx *FlowCtx
}
return true
}

// Prettier aliases for execinfrapb.ScanVisibility values.
const (
ScanVisibilityPublic = execinfrapb.ScanVisibility_PUBLIC
ScanVisibilityPublicAndNotPublic = execinfrapb.ScanVisibility_PUBLIC_AND_NOT_PUBLIC
)
Loading

0 comments on commit 6ecbc02

Please sign in to comment.