From 0b08540c8047a9b065b091f640c22a99a2cf963b Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Thu, 4 Nov 2021 17:20:15 -0400 Subject: [PATCH] opt: make FK ON UPDATE CASCADE input columns anonymous This commit anonymizes the columns in the input of a foreign key ON UPDATE CASCADE expression. This is safe because these columns can only be referenced by other expressions if they are update columns, and in that case, `mutationBuilder.addUpdateCascade` will give them a distinct name in the scope it produces. This change allows a special case added in #57153 to be removed, without failing the regression test. Release note: None --- pkg/sql/opt/optbuilder/fk_cascade.go | 17 +++++++++-------- pkg/sql/opt/optbuilder/update.go | 15 --------------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/pkg/sql/opt/optbuilder/fk_cascade.go b/pkg/sql/opt/optbuilder/fk_cascade.go index 72454e0faad6..4bb0be1a7f31 100644 --- a/pkg/sql/opt/optbuilder/fk_cascade.go +++ b/pkg/sql/opt/optbuilder/fk_cascade.go @@ -677,10 +677,6 @@ func (cb *onUpdateCascadeBuilder) Build( updateExprs[i] = &tree.UpdateExpr{} switch cb.action { case tree.Cascade: - // TODO(radu): This requires special code in addUpdateCols to - // prevent this scopeColumn from being duplicated in mb.outScope - // (see the addCol anonymous function in addUpdateCols). Find a - // cleaner way to handle this. updateExprs[i].Expr = &newValScopeCols[i] case tree.SetNull: updateExprs[i].Expr = tree.DNull @@ -871,12 +867,17 @@ func (b *Builder) buildUpdateCascadeMutationInput( outScope.expr, mutationInput, on, memo.EmptyJoinPrivate, ) // Append the columns from the right-hand side to the scope. - for i, col := range outCols { + // + // These columns can only be referenced if they are update columns. In that + // case, they are given distinct names in mutationBuilder.addUpdateCol, so + // we make them anonymous here. This prevents column name ambiguity in + // outScope. There is no need to attach a metadata name here because these + // columns have already been added to the metadata with a name above in the + // calls to md.AddColumn. + for _, col := range outCols { colMeta := md.ColumnMeta(col) - ord := fk.OriginColumnOrdinal(childTable, i%numFKCols) - c := childTable.Column(ord) outScope.cols = append(outScope.cols, scopeColumn{ - name: scopeColName(c.ColName()), + name: scopeColName(""), id: col, typ: colMeta.Type, }) diff --git a/pkg/sql/opt/optbuilder/update.go b/pkg/sql/opt/optbuilder/update.go index 96cf7574c2e0..01841d7d412f 100644 --- a/pkg/sql/opt/optbuilder/update.go +++ b/pkg/sql/opt/optbuilder/update.go @@ -212,21 +212,6 @@ func (mb *mutationBuilder) addUpdateCols(exprs tree.UpdateExprs) { } addCol := func(expr tree.Expr, targetColID opt.ColumnID) { - // If the expression is already a scopeColumn, we can skip creating a - // new scopeColumn and proceed with type checking and adding the column - // to the list of source columns to update. The expression can be a - // scopeColumn when addUpdateCols is called from the - // onUpdateCascadeBuilder while building foreign key cascading updates. - // - // The input scopeColumn is a pointer to a column in mb.outScope. It was - // copied by value to projectionsScope. The checkCol function mutates - // the name of projected columns, so we must lookup the column in - // projectionsScope so that the correct scopeColumn is renamed. - if scopeCol, ok := expr.(*scopeColumn); ok { - checkCol(projectionsScope.getColumn(scopeCol.id), targetColID) - return - } - // Allow right side of SET to be DEFAULT. if _, ok := expr.(tree.DefaultVal); ok { expr = mb.parseDefaultExpr(targetColID)