From 40860a8f7d4f215cf02d392dddd3eb6f704c8e64 Mon Sep 17 00:00:00 2001 From: Aditya Maru Date: Fri, 19 Jul 2019 15:15:45 -0400 Subject: [PATCH] sqlbase: Move ProcessColumns() from sql to sqlbase ProcessColumns returns a list of ColumnDescriptors when provided a list of columns names and their table desc. Previously, this method was defined as a planner method in sql as it was only used for INSERT statements. With IMPORT INTO also working towards allowing users to specify target columns to import data into, there is value in reusing this method. This change does not modify the method in any way. Release note: None --- pkg/sql/copy.go | 2 +- pkg/sql/insert.go | 49 +----------------------------- pkg/sql/sqlbase/column_resolver.go | 45 +++++++++++++++++++++++++++ pkg/sql/update.go | 2 +- pkg/sql/upsert.go | 2 +- 5 files changed, 49 insertions(+), 51 deletions(-) diff --git a/pkg/sql/copy.go b/pkg/sql/copy.go index e374eee3ace2..8a5f8e0867ff 100644 --- a/pkg/sql/copy.go +++ b/pkg/sql/copy.go @@ -114,7 +114,7 @@ func newCopyMachine( if err := c.p.CheckPrivilege(ctx, tableDesc, privilege.INSERT); err != nil { return nil, err } - cols, err := c.p.processColumns(tableDesc, n.Columns, + cols, err := sqlbase.ProcessTargetColumns(tableDesc, n.Columns, true /* ensureColumns */, false /* allowMutations */) if err != nil { return nil, err diff --git a/pkg/sql/insert.go b/pkg/sql/insert.go index 9e566d75db73..6e3084904b56 100644 --- a/pkg/sql/insert.go +++ b/pkg/sql/insert.go @@ -148,7 +148,7 @@ func (p *planner) Insert( insertCols = desc.WritableColumns() } else { var err error - if insertCols, err = p.processColumns(desc, n.Columns, + if insertCols, err = sqlbase.ProcessTargetColumns(desc, n.Columns, true /* ensureColumns */, false /* allowMutations */); err != nil { return nil, err } @@ -619,53 +619,6 @@ func (n *insertNode) enableAutoCommit() { n.run.ti.enableAutoCommit() } -// processColumns returns the column descriptors identified by the -// given name list. It also checks that a given column name is only -// listed once. If no column names are given (special case for INSERT) -// and ensureColumns is set, the descriptors for all visible columns -// are returned. If allowMutations is set, even columns undergoing -// mutations are added. -func (p *planner) processColumns( - tableDesc *sqlbase.ImmutableTableDescriptor, - nameList tree.NameList, - ensureColumns, allowMutations bool, -) ([]sqlbase.ColumnDescriptor, error) { - if len(nameList) == 0 { - if ensureColumns { - // VisibleColumns is used here to prevent INSERT INTO VALUES (...) - // (as opposed to INSERT INTO
(...) VALUES (...)) from writing - // hidden columns. At present, the only hidden column is the implicit rowid - // primary key column. - return tableDesc.VisibleColumns(), nil - } - return nil, nil - } - - cols := make([]sqlbase.ColumnDescriptor, len(nameList)) - colIDSet := make(map[sqlbase.ColumnID]struct{}, len(nameList)) - for i, colName := range nameList { - var col *sqlbase.ColumnDescriptor - var err error - if allowMutations { - col, _, err = tableDesc.FindColumnByName(colName) - } else { - col, err = tableDesc.FindActiveColumnByName(string(colName)) - } - if err != nil { - return nil, err - } - - if _, ok := colIDSet[col.ID]; ok { - return nil, pgerror.Newf(pgcode.Syntax, - "multiple assignments to the same column %q", &nameList[i]) - } - colIDSet[col.ID] = struct{}{} - cols[i] = *col - } - - return cols, nil -} - // extractInsertSource removes the parentheses around the data source of an INSERT statement. // If the data source is a VALUES clause not further qualified with LIMIT/OFFSET and ORDER BY, // the 3rd return value is a pre-casted pointer to the VALUES clause. diff --git a/pkg/sql/sqlbase/column_resolver.go b/pkg/sql/sqlbase/column_resolver.go index 063cb13b190e..86f0eb485636 100644 --- a/pkg/sql/sqlbase/column_resolver.go +++ b/pkg/sql/sqlbase/column_resolver.go @@ -20,6 +20,51 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" ) +// ProcessTargetColumns returns the column descriptors identified by the +// given name list. It also checks that a given column name is only +// listed once. If no column names are given (special case for INSERT) +// and ensureColumns is set, the descriptors for all visible columns +// are returned. If allowMutations is set, even columns undergoing +// mutations are added. +func ProcessTargetColumns( + tableDesc *ImmutableTableDescriptor, nameList tree.NameList, ensureColumns, allowMutations bool, +) ([]ColumnDescriptor, error) { + if len(nameList) == 0 { + if ensureColumns { + // VisibleColumns is used here to prevent INSERT INTO
VALUES (...) + // (as opposed to INSERT INTO
(...) VALUES (...)) from writing + // hidden columns. At present, the only hidden column is the implicit rowid + // primary key column. + return tableDesc.VisibleColumns(), nil + } + return nil, nil + } + + cols := make([]ColumnDescriptor, len(nameList)) + colIDSet := make(map[ColumnID]struct{}, len(nameList)) + for i, colName := range nameList { + var col *ColumnDescriptor + var err error + if allowMutations { + col, _, err = tableDesc.FindColumnByName(colName) + } else { + col, err = tableDesc.FindActiveColumnByName(string(colName)) + } + if err != nil { + return nil, err + } + + if _, ok := colIDSet[col.ID]; ok { + return nil, pgerror.Newf(pgcode.Syntax, + "multiple assignments to the same column %q", &nameList[i]) + } + colIDSet[col.ID] = struct{}{} + cols[i] = *col + } + + return cols, nil +} + // sourceNameMatches checks whether a request for table name toFind // can be satisfied by the FROM source name srcName. // diff --git a/pkg/sql/update.go b/pkg/sql/update.go index 1f9a67705565..e17582e19c87 100644 --- a/pkg/sql/update.go +++ b/pkg/sql/update.go @@ -128,7 +128,7 @@ func (p *planner) Update( // Extract the column descriptors for the column names listed // in the LHS operands of SET expressions. This also checks // that each column is assigned at most once. - updateCols, err := p.processColumns(desc, names, + updateCols, err := sqlbase.ProcessTargetColumns(desc, names, true /* ensureColumns */, false /* allowMutations */) if err != nil { return nil, err diff --git a/pkg/sql/upsert.go b/pkg/sql/upsert.go index a6363b94d682..592c68bc1bff 100644 --- a/pkg/sql/upsert.go +++ b/pkg/sql/upsert.go @@ -142,7 +142,7 @@ func (p *planner) newUpsertNode( // to include mutation columns being added. If the SET // expressions were explicit (specified by the client), // then we want to reject assignments to mutation columns. - updateCols, err := p.processColumns(desc, names, + updateCols, err := sqlbase.ProcessTargetColumns(desc, names, false /* ensureColumns */, autoGenUpdates /* allowMutations */) if err != nil { return nil, err