sql: partial predicate causes error for UPDATE .. FROM

[RaduBerinde]

A partial index predicate is generating an ambigous column error with UPDATE .. FROM, when the same column name is produced by the FROM relation. Example:

> create table t (a int primary key, b int, c int, index (b,c) where c > 0);
> update t set b=v.b, c=v.c from (values (1,1), (2,2)) as v(b,c) where t.b=v.b;
ERROR: column reference "c" is ambiguous (candidates: t.c, v.c)
SQLSTATE: 42702

The same statement works if where c > 0 is removed from the index.

[RaduBerinde]

The problem is the scope we are using to resolve partial index predicates.

In general, I believe things would be more robust if we'd build predicates once for the TableMeta and then remap the column IDs in that expression (with CopyAndReplace) according to strict column ID mappings (instead of relying on scope resolution by name). That would be a non-trivial project though (and probably not backportable).

[RaduBerinde]

Actually, it looks like the problem is when resolving the "del" columns. We are aliasing mb.outScope with mb.fetchScope and the latter ends up containing the FROM columns.

[RaduBerinde]

index 109e84646d..0bd2b5b662 100644
--- a/pkg/sql/opt/optbuilder/mutation_builder.go
+++ b/pkg/sql/opt/optbuilder/mutation_builder.go
@@ -287,10 +287,9 @@ func (mb *mutationBuilder) buildInputForUpdate(
 		noRowLocking,
 		inScope,
 	)
-	mb.outScope = mb.fetchScope
 
 	// Set list of columns that will be fetched by the input expression.
-	mb.setFetchColIDs(mb.outScope.cols)
+	mb.setFetchColIDs(mb.fetchScope.cols)
 
 	// If there is a FROM clause present, we must join all the tables
 	// together with the table being updated.
@@ -299,18 +298,24 @@ func (mb *mutationBuilder) buildInputForUpdate(
 		fromScope := mb.b.buildFromTables(from, noRowLocking, inScope)
 
 		// Check that the same table name is not used multiple times.
-		mb.b.validateJoinTableNames(mb.outScope, fromScope)
+		mb.b.validateJoinTableNames(mb.fetchScope, fromScope)
 
 		// The FROM table columns can be accessed by the RETURNING clause of the
 		// query and so we have to make them accessible.
 		mb.extraAccessibleCols = fromScope.cols
 
 		// Add the columns in the FROM scope.
+		// Note: we don't want to modify fetchScope here - it will be used later
+		// with partial index predicates and the FROM columns can cause ambiguities.
+		mb.outScope = mb.fetchScope.replace()
+		mb.outScope.appendColumnsFromScope(mb.fetchScope)
 		mb.outScope.appendColumnsFromScope(fromScope)
 
-		left := mb.outScope.expr.(memo.RelExpr)
+		left := mb.fetchScope.expr.(memo.RelExpr)
 		right := fromScope.expr.(memo.RelExpr)
 		mb.outScope.expr = mb.b.factory.ConstructInnerJoin(left, right, memo.TrueFilter, memo.EmptyJoinPrivate)
+	} else {
+		mb.outScope = mb.fetchScope
 	}

This fixes this particular case.

However, the "workaround attempt" in the support issue still doesn't work. That one has a problem when resolving with the PUT scope. The problem there seems to be that the "round decimal values" code interacts badly with the code that renames the "new" columns in addUpdateCols. Because the updateColIds[] map is changed by roundDecimalValues and the new column does not have the expected name.

[RaduBerinde]

This second problem seems to be fixed by this:

 		mb.roundedDecimalCols.Add(scopeCol.id)
+		scopeCol.name = mb.tab.Column(i).ColName()

However, this all still feels very fragile; I filed separate PR #61298 to investigate a better mechanism. In the meantime, we'll need to fix in 20.2 and add tests with UPDATE .. FROM and partial indexes.

[awoods187]

Nice analysis!

[mgartner]

Thanks for hunting this down @RaduBerinde! I'll work on a fixing this today.

[mgartner]

The "second problem" is also present with check constraints, not just partial indexes. It's a harder problem to solve, so I've opened #61520 to track it. I'm experimenting with some potential fixes.