Skip to content

Commit

Permalink
sql: stop returning dropped unique columns in DELETE RETURNING
Browse files Browse the repository at this point in the history
this change is a followup to fix #32188

unique columns have an index on them. When such a column is dropped,
a DELETE statement will read a dropped column to correctly drop
the index on it, but this dropped column is exposed through the
RETURNING clause. This change stops exposing the column via RETURNING.

fixes #33361

Release note (sql change): fixed problem with returning dropped
unique columns in DELETE RETURNING statement.
  • Loading branch information
vivekmenezes committed Jan 2, 2019
1 parent e8c097d commit 0023c78
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
22 changes: 21 additions & 1 deletion pkg/sql/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,18 @@ func (p *planner) Delete(
var columns sqlbase.ResultColumns
if rowsNeeded {
columns = planColumns(rows)
// If rowsNeeded is set, we have requested from the source above
// all the columns from the descriptor. However, to ensure that
// modified rows include all columns, the construction of the
// source has used publicAndNonPublicColumns so the source may
// contain additional columns for every newly dropped column not
// visible.
// We do not want these to be available for RETURNING below.
//
// In the case where rowsNeeded is true, the requested columns are
// requestedCols. So we can truncate to the length of that to
// only see public columns.
columns = columns[:len(requestedCols)]
}

// Now make a delete node. We use a pool.
Expand Down Expand Up @@ -326,7 +338,15 @@ func (d *deleteNode) processSourceRow(params runParams, sourceVals tree.Datums)

// If result rows need to be accumulated, do it.
if d.run.rows != nil {
if _, err := d.run.rows.AddRow(params.ctx, sourceVals); err != 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.
//
// d.columns is guaranteed to only contain the requested
// public columns.
resultValues := sourceVals[:len(d.columns)]
if _, err := d.run.rows.AddRow(params.ctx, resultValues); err != nil {
return err
}
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/delete
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,25 @@ DELETE FROM t29494 RETURNING *

statement ok
COMMIT

subtest regression_33361

statement ok
CREATE TABLE t33361(x INT PRIMARY KEY, y INT UNIQUE, z INT); INSERT INTO t33361 VALUES (1, 2, 3)

statement ok
BEGIN; ALTER TABLE t33361 DROP COLUMN y

statement error column "y" does not exist
DELETE FROM t33361 RETURNING y

statement ok
ROLLBACK

statement ok
BEGIN; ALTER TABLE t33361 DROP COLUMN y

query II
DELETE FROM t33361 RETURNING *; COMMIT
----
1 3

0 comments on commit 0023c78

Please sign in to comment.