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: stop returning dropped unique columns in DELETE RETURNING #33438

Merged
merged 1 commit into from
Jan 3, 2019
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
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