Re: Alias collision in `refresh materialized view concurrently`
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
Thomas Munro <thomas.munro@gmail.com>,
Bernd Helmle <mailings@oopsware.de>,
Mathis Rudolf <mathis.rudolf@credativ.de>,
PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-08-06T14:48:40Z
Lists: pgsql-hackers
Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Jun 02, 2021 at 03:44:45PM +0530, Bharath Rupireddy wrote:
>> Thanks.The changes with that approach are very minimal. PSA v5 and let
>> me know if any more changes are needed.
> Simple enough, so applied and back-patched.
I just came across this issue while preparing the release notes.
ISTM that people have expended a great deal of effort on a fundamentally
unreliable solution, when a reliable one is easily available.
The originally complained-of issue was that a user-chosen column name
could collide with the query-chosen table name:
ERROR: column reference "mv" is ambiguous
LINE 1: ...alog.=) mv.mv AND newdata OPERATOR(pg_catalog.*=) mv) WHERE ...
This is true, but it's self-inflicted damage, because all you have
to do is write the query so that mv is clearly a table name:
... mv.mv AND newdata.* OPERATOR(pg_catalog.*=) mv.*) WHERE ...
AFAICT that works and generates the identical parse tree to the original
coding. The only place touched by the patch where it's a bit difficult to
make the syntax unambiguous this way is
"CREATE TEMP TABLE %s AS "
"SELECT _$mv.ctid AS tid, _$newdata "
"FROM %s _$mv FULL JOIN %s _$newdata ON (",
because newdata.* would ordinarily get expanded to multiple columns
if it's at the top level of a SELECT list, and that's not what we want.
However, that's easily fixed using the same hack as in ruleutils.c's
get_variable: add a no-op cast to the table's rowtype. So this
would look like
appendStringInfo(&querybuf,
"CREATE TEMP TABLE %s AS "
"SELECT mv.ctid AS tid, newdata.*::%s "
"FROM %s mv FULL JOIN %s newdata ON (",
diffname, matviewname, matviewname, tempname);
Given that it took this long to notice the problem at all, maybe
this is not a fix to cram in on the weekend before the release wrap.
But I don't see why we need to settle for "mostly works" when
"always works" is barely any harder.
regards, tom lane
Commits
-
Really fix the ambiguity in REFRESH MATERIALIZED VIEW CONCURRENTLY.
- d33fc411000e 11.13 landed
- c08b3a9eb6cc 9.6.23 landed
- ba9f665a4413 13.4 landed
- ac818984a222 10.18 landed
- 9179a82d7af3 15.0 landed
- 2c915905e3e8 14.0 landed
- 1ff1e4a60646 12.8 landed
-
Reduce risks of conflicts in internal queries of REFRESH MATVIEW CONCURRENTLY
- d9525c46c89d 9.6.23 landed
- 20f70f558172 10.18 landed
- dbc9dbba5652 11.13 landed
- 4ceaa760bd8c 12.8 landed
- 75d66d10e0e3 13.4 landed
- 187682c32173 14.0 landed