Hello,we had a Customer-Report in which `refresh materialized view CONCURRENTLY` failed with: `ERROR: column reference "mv" is ambiguous`
They're using `mv` as an alias for one column and this is causing a collision with an internal alias. They also made it reproducible like this:
``` create materialized view testmv as select 'asdas' mv; --ok create unique index on testmv (mv); --ok refresh materialized view testmv; --ok refresh materialized view CONCURRENTLY testmv; ---BAM! ``` ``` ERROR: column reference "mv" is ambiguous LINE 1: ...alog.=) mv.mv AND newdata OPERATOR(pg_catalog.*=) mv) WHERE ... ^QUERY: CREATE TEMP TABLE pg_temp_4.pg_temp_218322_2 AS SELECT mv.ctid AS tid, newdata FROM public.testmv mv FULL JOIN pg_temp_4.pg_temp_218322 newdata ON (newdata.mv OPERATOR(pg_catalog.=) mv.mv AND newdata OPERATOR(pg_catalog.*=) mv) WHERE newdata IS NULL OR mv IS NULL ORDER BY tid
```The corresponding Code is in `matview.c` in function `refresh_by_match_merge`. With adding a prefix like `_pg_internal_` we could make collisions pretty unlikely, without intrusive changes.
The appended patch does this change for the aliases `mv`, `newdata` and `newdata2`.
Kind regards, Mathis
From 121b06258b80e5097c963335794e9a89f7e6d3ec Mon Sep 17 00:00:00 2001 From: Mathis Rudolf <mathis.rud...@credativ.de> Date: Mon, 17 May 2021 14:55:49 +0200 Subject: [PATCH] Fix alias collision for REFRESH MV CONCURRENTLY This patch adds the prefix _pg_internal_ to aliases like 'mv' and 'newdata' in 'refresh_by_match_merge()', which makes it unliky to cause any collisions with user created MVs. --- src/backend/commands/matview.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 172ec6e982..04602dba80 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -629,12 +629,12 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "SELECT newdata FROM %s newdata " - "WHERE newdata IS NOT NULL AND EXISTS " - "(SELECT 1 FROM %s newdata2 WHERE newdata2 IS NOT NULL " - "AND newdata2 OPERATOR(pg_catalog.*=) newdata " - "AND newdata2.ctid OPERATOR(pg_catalog.<>) " - "newdata.ctid)", + "SELECT pg_internal_newdata FROM %s pg_internal_newdata " + "WHERE pg_internal_newdata IS NOT NULL AND EXISTS " + "(SELECT 1 FROM %s pg_internal_newdata2 WHERE pg_internal_newdata2 IS NOT NULL " + "AND pg_internal_newdata2 OPERATOR(pg_catalog.*=) pg_internal_newdata " + "AND pg_internal_newdata2.ctid OPERATOR(pg_catalog.<>) " + "pg_internal_newdata.ctid)", tempname, tempname); if (SPI_execute(querybuf.data, false, 1) != SPI_OK_SELECT) elog(ERROR, "SPI_exec failed: %s", querybuf.data); @@ -662,8 +662,8 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, resetStringInfo(&querybuf); appendStringInfo(&querybuf, "CREATE TEMP TABLE %s AS " - "SELECT mv.ctid AS tid, newdata " - "FROM %s mv FULL JOIN %s newdata ON (", + "SELECT pg_internal_mv.ctid AS tid, pg_internal_newdata " + "FROM %s pg_internal_mv FULL JOIN %s pg_internal_newdata ON (", diffname, matviewname, tempname); /* @@ -756,9 +756,9 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, if (foundUniqueIndex) appendStringInfoString(&querybuf, " AND "); - leftop = quote_qualified_identifier("newdata", + leftop = quote_qualified_identifier("pg_internal_newdata", NameStr(attr->attname)); - rightop = quote_qualified_identifier("mv", + rightop = quote_qualified_identifier("pg_internal_mv", NameStr(attr->attname)); generate_operator_clause(&querybuf, @@ -786,8 +786,8 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, Assert(foundUniqueIndex); appendStringInfoString(&querybuf, - " AND newdata OPERATOR(pg_catalog.*=) mv) " - "WHERE newdata IS NULL OR mv IS NULL " + " AND pg_internal_newdata OPERATOR(pg_catalog.*=) pg_internal_mv) " + "WHERE pg_internal_newdata IS NULL OR pg_internal_mv IS NULL " "ORDER BY tid"); /* Create the temporary "diff" table. */ @@ -813,10 +813,10 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, /* Deletes must come before inserts; do them first. */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "DELETE FROM %s mv WHERE ctid OPERATOR(pg_catalog.=) ANY " + "DELETE FROM %s pg_internal_mv WHERE ctid OPERATOR(pg_catalog.=) ANY " "(SELECT diff.tid FROM %s diff " "WHERE diff.tid IS NOT NULL " - "AND diff.newdata IS NULL)", + "AND diff.pg_internal_newdata IS NULL)", matviewname, diffname); if (SPI_exec(querybuf.data, 0) != SPI_OK_DELETE) elog(ERROR, "SPI_exec failed: %s", querybuf.data); @@ -824,7 +824,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, /* Inserts go last. */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "INSERT INTO %s SELECT (diff.newdata).* " + "INSERT INTO %s SELECT (diff.pg_internal_newdata).* " "FROM %s diff WHERE tid IS NULL", matviewname, diffname); if (SPI_exec(querybuf.data, 0) != SPI_OK_INSERT) -- 2.31.1
OpenPGP_signature
Description: OpenPGP digital signature