Skip to content

Commit

Permalink
sql: remove type width enforcement during execution
Browse files Browse the repository at this point in the history
Assignment casts are now responsible for ensuring that a value written
to a column has a type and width that match the column type. This commit
removes the logic that performed this validation before assignment casts
existed.

Release note: None
  • Loading branch information
mgartner committed Feb 7, 2022
1 parent 7b48b61 commit cc24310
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 38 deletions.
2 changes: 1 addition & 1 deletion pkg/sql/insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (r *insertRun) initRowContainer(params runParams, columns colinfo.ResultCol
// processSourceRow processes one row from the source for insertion and, if
// result rows are needed, saves it in the result row container.
func (r *insertRun) processSourceRow(params runParams, rowVals tree.Datums) error {
if err := enforceLocalColumnConstraints(rowVals, r.insertCols, false /* isUpdate */); err != nil {
if err := enforceLocalColumnConstraints(rowVals, r.insertCols); err != nil {
return err
}

Expand Down
15 changes: 3 additions & 12 deletions pkg/sql/row/row_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,18 +148,9 @@ func GenerateInsertRow(

// Verify the column constraints.
//
// We would really like to use enforceLocalColumnConstraints() here,
// but this is not possible because of some brain damage in the
// Insert() constructor, which causes insertCols to contain
// duplicate columns descriptors: computed columns are listed twice,
// one will receive a NULL value and one will receive a comptued
// value during execution. It "works out in the end" because the
// latter (non-NULL) value overwrites the earlier, but
// enforceLocalColumnConstraints() does not know how to reason about
// this.
//
// In the end it does not matter much, this code is going away in
// favor of the (simpler, correct) code in the CBO.
// During mutations (INSERT, UPDATE, UPSERT), this is checked by
// sql.enforceLocalColumnConstraints. These checks are required for IMPORT
// statements.

// Check to see if NULL is being inserted into any non-nullable column.
for _, col := range tableDesc.WritableColumns() {
Expand Down
6 changes: 1 addition & 5 deletions pkg/sql/tablewriter_upsert_opt.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,7 @@ func (tu *optTableUpserter) updateConflictingRow(
// via GenerateInsertRow().
// - for the fetched part, we assume that the data in the table is
// correct already.
if err := enforceLocalColumnConstraints(
updateValues,
tu.updateCols,
true, /* isUpdate */
); err != nil {
if err := enforceLocalColumnConstraints(updateValues, tu.updateCols); err != nil {
return err
}

Expand Down
17 changes: 2 additions & 15 deletions pkg/sql/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,7 @@ func (u *updateNode) processSourceRow(params runParams, sourceVals tree.Datums)
// Verify the schema constraints. For consistency with INSERT/UPSERT
// and compatibility with PostgreSQL, we must do this before
// processing the CHECK constraints.
if err := enforceLocalColumnConstraints(
u.run.updateValues,
u.run.tu.ru.UpdateCols,
true, /* isUpdate */
); err != nil {
if err := enforceLocalColumnConstraints(u.run.updateValues, u.run.tu.ru.UpdateCols); err != nil {
return err
}

Expand Down Expand Up @@ -416,20 +412,11 @@ func (ss scalarSlot) checkColumnTypes(row []tree.TypedExpr) error {
// enforceLocalColumnConstraints asserts the column constraints that do not
// require data validation from other sources than the row data itself. This
// currently only includes checking for null values in non-nullable columns.
func enforceLocalColumnConstraints(row tree.Datums, cols []catalog.Column, isUpdate bool) error {
func enforceLocalColumnConstraints(row tree.Datums, cols []catalog.Column) error {
for i, col := range cols {
if !col.IsNullable() && row[i] == tree.DNull {
return sqlerrors.NewNonNullViolationError(col.GetName())
}
if isUpdate {
// TODO(mgartner): Remove this once assignment casts are supported
// for UPSERTs and UPDATEs.
outVal, err := tree.AdjustValueToType(col.GetType(), row[i])
if err != nil {
return err
}
row[i] = outVal
}
}
return nil
}
6 changes: 1 addition & 5 deletions pkg/sql/upsert.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,7 @@ func (n *upsertNode) BatchedNext(params runParams) (bool, error) {
// processSourceRow processes one row from the source for upsertion.
// The table writer is in charge of accumulating the result rows.
func (n *upsertNode) processSourceRow(params runParams, rowVals tree.Datums) error {
if err := enforceLocalColumnConstraints(
rowVals,
n.run.insertCols,
true, /* isUpdate */
); err != nil {
if err := enforceLocalColumnConstraints(rowVals, n.run.insertCols); err != nil {
return err
}

Expand Down

0 comments on commit cc24310

Please sign in to comment.