diff options
| -rw-r--r-- | src/backend/commands/matview.c | 51 | ||||
| -rw-r--r-- | src/test/regress/expected/matview.out | 10 | ||||
| -rw-r--r-- | src/test/regress/sql/matview.sql | 10 |
3 files changed, 47 insertions, 24 deletions
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 52c37901959..80e9ec02864 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -529,9 +529,12 @@ transientrel_destroy(DestReceiver *self) /* * Given a qualified temporary table name, append an underscore followed by * the given integer, to make a new table name based on the old one. + * The result is a palloc'd string. * - * This leaks memory through palloc(), which won't be cleaned up until the - * current memory context is freed. + * As coded, this would fail to make a valid SQL name if the given name were, + * say, "FOO"."BAR". Currently, the table name portion of the input will + * never be double-quoted because it's of the form "pg_temp_NNN", cf + * make_new_heap(). But we might have to work harder someday. */ static char * make_temptable_name_n(char *tempname, int n) @@ -619,16 +622,20 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, * that in a way that allows showing the first duplicated row found. Even * after we pass this test, a unique index on the materialized view may * find a duplicate key problem. + * + * Note: here and below, we use "tablename.*::tablerowtype" as a hack to + * keep ".*" from being expanded into multiple columns in a SELECT list. + * Compare ruleutils.c's get_variable(). */ 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)", - tempname, tempname); + "SELECT newdata.*::%s 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)", + tempname, tempname, tempname); if (SPI_execute(querybuf.data, false, 1) != SPI_OK_SELECT) elog(ERROR, "SPI_exec failed: %s", querybuf.data); if (SPI_processed > 0) @@ -655,9 +662,9 @@ 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 (", - diffname, matviewname, tempname); + "SELECT mv.ctid AS tid, newdata.*::%s AS newdata " + "FROM %s mv FULL JOIN %s newdata ON (", + diffname, tempname, matviewname, tempname); /* * Get the list of index OIDs for the table from the relcache, and look up @@ -749,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("newdata", NameStr(attr->attname)); - rightop = quote_qualified_identifier("_$mv", + rightop = quote_qualified_identifier("mv", NameStr(attr->attname)); generate_operator_clause(&querybuf, @@ -779,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 newdata.* OPERATOR(pg_catalog.*=) mv.*) " + "WHERE newdata.* IS NULL OR mv.* IS NULL " "ORDER BY tid"); /* Create the temporary "diff" table. */ @@ -806,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 " - "(SELECT _$diff.tid FROM %s _$diff " - "WHERE _$diff.tid IS NOT NULL " - "AND _$diff._$newdata IS NULL)", + "DELETE FROM %s mv WHERE ctid OPERATOR(pg_catalog.=) ANY " + "(SELECT diff.tid FROM %s diff " + "WHERE diff.tid IS NOT NULL " + "AND diff.newdata IS NULL)", matviewname, diffname); if (SPI_exec(querybuf.data, 0) != SPI_OK_DELETE) elog(ERROR, "SPI_exec failed: %s", querybuf.data); @@ -817,8 +824,8 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, /* Inserts go last. */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "INSERT INTO %s SELECT (_$diff._$newdata).* " - "FROM %s _$diff WHERE tid IS NULL", + "INSERT INTO %s SELECT (diff.newdata).* " + "FROM %s diff WHERE tid IS NULL", matviewname, diffname); if (SPI_exec(querybuf.data, 0) != SPI_OK_INSERT) elog(ERROR, "SPI_exec failed: %s", querybuf.data); diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out index d0121a7b0b8..0af0ba61bcf 100644 --- a/src/test/regress/expected/matview.out +++ b/src/test/regress/expected/matview.out @@ -551,7 +551,15 @@ NOTICE: drop cascades to materialized view mvtest_mv_v -- make sure running as superuser works when MV owned by another role (bug #11208) CREATE ROLE regress_user_mvtest; SET ROLE regress_user_mvtest; -CREATE TABLE mvtest_foo_data AS SELECT i, md5(random()::text) +-- this test case also checks for ambiguity in the queries issued by +-- refresh_by_match_merge(), by choosing column names that intentionally +-- duplicate all the aliases used in those queries +CREATE TABLE mvtest_foo_data AS SELECT i, + i+1 AS tid, + md5(random()::text) AS mv, + md5(random()::text) AS newdata, + md5(random()::text) AS newdata2, + md5(random()::text) AS diff FROM generate_series(1, 10) i; CREATE MATERIALIZED VIEW mvtest_mv_foo AS SELECT * FROM mvtest_foo_data; CREATE MATERIALIZED VIEW mvtest_mv_foo AS SELECT * FROM mvtest_foo_data; diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql index d96175aa26c..1d690254641 100644 --- a/src/test/regress/sql/matview.sql +++ b/src/test/regress/sql/matview.sql @@ -211,7 +211,15 @@ DROP TABLE mvtest_v CASCADE; -- make sure running as superuser works when MV owned by another role (bug #11208) CREATE ROLE regress_user_mvtest; SET ROLE regress_user_mvtest; -CREATE TABLE mvtest_foo_data AS SELECT i, md5(random()::text) +-- this test case also checks for ambiguity in the queries issued by +-- refresh_by_match_merge(), by choosing column names that intentionally +-- duplicate all the aliases used in those queries +CREATE TABLE mvtest_foo_data AS SELECT i, + i+1 AS tid, + md5(random()::text) AS mv, + md5(random()::text) AS newdata, + md5(random()::text) AS newdata2, + md5(random()::text) AS diff FROM generate_series(1, 10) i; CREATE MATERIALIZED VIEW mvtest_mv_foo AS SELECT * FROM mvtest_foo_data; CREATE MATERIALIZED VIEW mvtest_mv_foo AS SELECT * FROM mvtest_foo_data; |