Re: Alias collision in `refresh materialized view concurrently`

On Wed, May 19, 2021 at 5:33 PM Mathis Rudolf <mathis(dot)rudolf(at)credativ(dot)de> wrote:
>
> 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`.

I think it's better to have some random name, see below. We could
either use "OIDNewHeap" or "MyBackendId" to make those column names
unique and almost unguessable. So, something like "pg_temp1_XXXX",
"pg_temp2_XXXX" or "pg_temp3_XXXX" and so on would be better IMO.

snprintf(NewHeapName, sizeof(NewHeapName), "pg_temp_%u", OIDOldHeap);
snprintf(namespaceName, sizeof(namespaceName), "pg_temp_%d", MyBackendId);

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com