Re: Alias collision in `refresh materialized view concurrently` - Mailing list pgsql-hackers

From Bernd Helmle
Subject Re: Alias collision in `refresh materialized view concurrently`
Date
Msg-id cd8ac651b1e7f41bb3b17d98c766e6e9b14f8e28.camel@oopsware.de
Whole thread Raw
In response to Re: Alias collision in `refresh materialized view concurrently`  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Alias collision in `refresh materialized view concurrently`
List pgsql-hackers
Am Mittwoch, dem 19.05.2021 um 18:06 +0530 schrieb Bharath Rupireddy:
> > 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);

Hmm, it's an idea, but this can also lead to pretty random failures if
you have an unlucky user who had the same idea in its generating query
tool than the backend, no? Not sure if that's really better.

With the current implementation of REFRESH MATERIALIZED VIEW
CONCURRENTLY we always have the problem of possible collisions here,
you'd never get out of this area without analyzing the whole query for
such collisions. 

"mv" looks like a very common alias (i use it all over the time when
testing or playing around with materialized views, so i'm wondering why
i didn't see this issue already myself). So the risk here for such a
collision looks very high. We can try to lower this risk by choosing an
alias name, which is not so common. With a static alias however you get
a static error condition, not something that fails here and then.


-- 
Thanks,
    Bernd





pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: multi-install PostgresNode fails with older postgres versions
Next
From: "tsunakawa.takay@fujitsu.com"
Date:
Subject: RE: Bug in query rewriter - hasModifyingCTE not getting set