Tab completion for ALTER MATERIALIZED VIEW ... SET ACCESS METHOD
Hello hackers,
SET ACCESS METHOD is supported in ALTER TABLE since the commit
b0483263dd. Since that time, this also has be allowed SET ACCESS
METHOD in ALTER MATERIALIZED VIEW. Although it is not documented,
this works.
I cannot found any reasons to prohibit SET ACCESS METHOD in ALTER
MATERIALIZED VIEW, so I think it is better to support this in psql
tab-completion and be documented.
I attached a patch to fix the tab-completion and the documentation
about this syntax. Also, I added description about SET TABLESPACE
syntax that would have been overlooked.
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata@sraoss.co.jp>
Attachments:
tab_complete_alter_matview.patchtext/x-diff; name=tab_complete_alter_matview.patchDownload
diff --git a/doc/src/sgml/ref/alter_materialized_view.sgml b/doc/src/sgml/ref/alter_materialized_view.sgml
index 7011a0e7da..cae135c27a 100644
--- a/doc/src/sgml/ref/alter_materialized_view.sgml
+++ b/doc/src/sgml/ref/alter_materialized_view.sgml
@@ -43,6 +43,8 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE <replaceable class="parameter">name</r
ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET COMPRESSION <replaceable class="parameter">compression_method</replaceable>
CLUSTER ON <replaceable class="parameter">index_name</replaceable>
SET WITHOUT CLUSTER
+ SET ACCESS METHOD <replaceable class="parameter">new_access_method</replaceable>
+ SET TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
SET ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
RESET ( <replaceable class="parameter">storage_parameter</replaceable> [, ... ] )
OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_ROLE | CURRENT_USER | SESSION_USER }
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 6957567264..daf4206caa 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2124,7 +2124,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("TO");
/* ALTER MATERIALIZED VIEW xxx SET */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "SET"))
- COMPLETE_WITH("(", "SCHEMA", "TABLESPACE", "WITHOUT CLUSTER");
+ COMPLETE_WITH("(", "ACCESS METHOD", "SCHEMA", "TABLESPACE", "WITHOUT CLUSTER");
/* ALTER POLICY <name> */
else if (Matches("ALTER", "POLICY"))
COMPLETE_WITH_QUERY(Query_for_list_of_policies);
@@ -2485,6 +2485,13 @@ psql_completion(const char *text, int start, int end)
else if (Matches("ALTER", "TYPE", MatchAny, "RENAME", "VALUE"))
COMPLETE_WITH_ENUM_VALUE(prev3_wd);
+ /*
+ * If we have ALTER MATERIALIZED VIEW <sth> SET ACCESS METHOD provide a list of table
+ * AMs.
+ */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "SET", "ACCESS", "METHOD"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_table_access_methods);
+
/*
* ANALYZE [ ( option [, ...] ) ] [ table_and_columns [, ...] ]
* ANALYZE [ VERBOSE ] [ table_and_columns [, ...] ]
Hi Nagata-san,
On Wed, Mar 16, 2022 at 01:33:37PM +0900, Yugo NAGATA wrote:
SET ACCESS METHOD is supported in ALTER TABLE since the commit
b0483263dd. Since that time, this also has be allowed SET ACCESS
METHOD in ALTER MATERIALIZED VIEW. Although it is not documented,
this works.
Yes, that's an oversight. I see no reason to not authorize that, and
the rewrite path in tablecmds.c is the same as for plain tables.
I cannot found any reasons to prohibit SET ACCESS METHOD in ALTER
MATERIALIZED VIEW, so I think it is better to support this in psql
tab-completion and be documented.
I think that we should have some regression tests about those command
flavors. How about adding a couple of queries to create_am.sql for
SET ACCESS METHOD and to tablespace.sql for SET TABLESPACE?
--
Michael
Hi Michael-san,
On Wed, 16 Mar 2022 16:18:09 +0900
Michael Paquier <michael@paquier.xyz> wrote:
Hi Nagata-san,
On Wed, Mar 16, 2022 at 01:33:37PM +0900, Yugo NAGATA wrote:
SET ACCESS METHOD is supported in ALTER TABLE since the commit
b0483263dd. Since that time, this also has be allowed SET ACCESS
METHOD in ALTER MATERIALIZED VIEW. Although it is not documented,
this works.Yes, that's an oversight. I see no reason to not authorize that, and
the rewrite path in tablecmds.c is the same as for plain tables.I cannot found any reasons to prohibit SET ACCESS METHOD in ALTER
MATERIALIZED VIEW, so I think it is better to support this in psql
tab-completion and be documented.I think that we should have some regression tests about those command
flavors. How about adding a couple of queries to create_am.sql for
SET ACCESS METHOD and to tablespace.sql for SET TABLESPACE?
Thank you for your review!
I added some queries in the regression test. Attached is the updated patch.
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata@sraoss.co.jp>
Attachments:
v2_tab_complete_alter_matview.patchtext/x-diff; name=v2_tab_complete_alter_matview.patchDownload
diff --git a/doc/src/sgml/ref/alter_materialized_view.sgml b/doc/src/sgml/ref/alter_materialized_view.sgml
index 7011a0e7da..cae135c27a 100644
--- a/doc/src/sgml/ref/alter_materialized_view.sgml
+++ b/doc/src/sgml/ref/alter_materialized_view.sgml
@@ -43,6 +43,8 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE <replaceable class="parameter">name</r
ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET COMPRESSION <replaceable class="parameter">compression_method</replaceable>
CLUSTER ON <replaceable class="parameter">index_name</replaceable>
SET WITHOUT CLUSTER
+ SET ACCESS METHOD <replaceable class="parameter">new_access_method</replaceable>
+ SET TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
SET ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] )
RESET ( <replaceable class="parameter">storage_parameter</replaceable> [, ... ] )
OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_ROLE | CURRENT_USER | SESSION_USER }
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 380cbc0b1f..c3fddcd0af 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2124,7 +2124,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("TO");
/* ALTER MATERIALIZED VIEW xxx SET */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "SET"))
- COMPLETE_WITH("(", "SCHEMA", "TABLESPACE", "WITHOUT CLUSTER");
+ COMPLETE_WITH("(", "ACCESS METHOD", "SCHEMA", "TABLESPACE", "WITHOUT CLUSTER");
/* ALTER POLICY <name> */
else if (Matches("ALTER", "POLICY"))
COMPLETE_WITH_QUERY(Query_for_list_of_policies);
@@ -2485,6 +2485,13 @@ psql_completion(const char *text, int start, int end)
else if (Matches("ALTER", "TYPE", MatchAny, "RENAME", "VALUE"))
COMPLETE_WITH_ENUM_VALUE(prev3_wd);
+ /*
+ * If we have ALTER MATERIALIZED VIEW <sth> SET ACCESS METHOD provide a list of table
+ * AMs.
+ */
+ else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny, "SET", "ACCESS", "METHOD"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_table_access_methods);
+
/*
* ANALYZE [ ( option [, ...] ) ] [ table_and_columns [, ...] ]
* ANALYZE [ VERBOSE ] [ table_and_columns [, ...] ]
diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out
index 32b7134080..cf83c75ab3 100644
--- a/src/test/regress/expected/create_am.out
+++ b/src/test/regress/expected/create_am.out
@@ -254,9 +254,33 @@ SELECT COUNT(a), COUNT(1) FILTER(WHERE a=1) FROM heaptable;
9 | 1
(1 row)
+-- ALTER MATERIALIZED VIEW SET ACCESS METHOD
+CREATE MATERIALIZED VIEW heapmv USING heap AS SELECt * FROM heaptable;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heapmv'::regclass;
+ amname
+--------
+ heap
+(1 row)
+
+ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap2;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heapmv'::regclass;
+ amname
+--------
+ heap2
+(1 row)
+
+SELECT COUNT(a), COUNT(1) FILTER(WHERE a=1) FROM heapmv;
+ count | count
+-------+-------
+ 9 | 1
+(1 row)
+
-- No support for multiple subcommands
ALTER TABLE heaptable SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
ERROR: cannot have multiple SET ACCESS METHOD subcommands
+DROP MATERIALIZED VIEW heapmv;
DROP TABLE heaptable;
-- No support for partitioned tables.
CREATE TABLE am_partitioned(x INT, y INT)
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index 473fe8c28e..c52cf1cfcf 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -905,6 +905,16 @@ SELECT COUNT(*) FROM testschema.atable; -- checks heap
3
(1 row)
+-- let's try moving a materialized view from one place to another
+CREATE MATERIALIZED VIEW testschema.amv AS SELECT * FROM testschema.atable;
+ALTER MATERIALIZED VIEW testschema.amv SET TABLESPACE regress_tblspace;
+REFRESH MATERIALIZED VIEW testschema.amv;
+SELECT COUNT(*) FROM testschema.amv;
+ count
+-------
+ 3
+(1 row)
+
-- Will fail with bad path
CREATE TABLESPACE regress_badspace LOCATION '/no/such/location';
ERROR: directory "/no/such/location" does not exist
@@ -939,18 +949,22 @@ RESET ROLE;
ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
ALTER INDEX ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
+ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
-- Should show notice that nothing was done
ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found
+ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
+NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found
-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
DROP SCHEMA testschema CASCADE;
-NOTICE: drop cascades to 6 other objects
+NOTICE: drop cascades to 7 other objects
DETAIL: drop cascades to table testschema.foo
drop cascades to table testschema.asselect
drop cascades to table testschema.asexecute
drop cascades to table testschema.part
drop cascades to table testschema.atable
+drop cascades to materialized view testschema.amv
drop cascades to table testschema.tablespace_acl
DROP ROLE regress_tablespace_user1;
DROP ROLE regress_tablespace_user2;
diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql
index 967bfac21a..c642448905 100644
--- a/src/test/regress/sql/create_am.sql
+++ b/src/test/regress/sql/create_am.sql
@@ -170,8 +170,17 @@ ALTER TABLE heaptable SET ACCESS METHOD heap2;
SELECT amname FROM pg_class c, pg_am am
WHERE c.relam = am.oid AND c.oid = 'heaptable'::regclass;
SELECT COUNT(a), COUNT(1) FILTER(WHERE a=1) FROM heaptable;
+-- ALTER MATERIALIZED VIEW SET ACCESS METHOD
+CREATE MATERIALIZED VIEW heapmv USING heap AS SELECt * FROM heaptable;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heapmv'::regclass;
+ALTER MATERIALIZED VIEW heapmv SET ACCESS METHOD heap2;
+SELECT amname FROM pg_class c, pg_am am
+ WHERE c.relam = am.oid AND c.oid = 'heapmv'::regclass;
+SELECT COUNT(a), COUNT(1) FILTER(WHERE a=1) FROM heapmv;
-- No support for multiple subcommands
ALTER TABLE heaptable SET ACCESS METHOD heap, SET ACCESS METHOD heap2;
+DROP MATERIALIZED VIEW heapmv;
DROP TABLE heaptable;
-- No support for partitioned tables.
CREATE TABLE am_partitioned(x INT, y INT)
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index 0949a28488..21db433f2a 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -380,6 +380,12 @@ INSERT INTO testschema.atable VALUES(3); -- ok
INSERT INTO testschema.atable VALUES(1); -- fail (checks index)
SELECT COUNT(*) FROM testschema.atable; -- checks heap
+-- let's try moving a materialized view from one place to another
+CREATE MATERIALIZED VIEW testschema.amv AS SELECT * FROM testschema.atable;
+ALTER MATERIALIZED VIEW testschema.amv SET TABLESPACE regress_tblspace;
+REFRESH MATERIALIZED VIEW testschema.amv;
+SELECT COUNT(*) FROM testschema.amv;
+
-- Will fail with bad path
CREATE TABLESPACE regress_badspace LOCATION '/no/such/location';
@@ -414,9 +420,11 @@ ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
ALTER INDEX ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
+ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
-- Should show notice that nothing was done
ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
+ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
On Fri, Mar 18, 2022 at 10:27:41AM +0900, Yugo NAGATA wrote:
I added some queries in the regression test. Attached is the updated patch.
Thanks. This looks rather sane to me. I'll split things into 3
commits in total, as of the psql completion, SET TABLESPACE and SET
ACCESS METHOD. The first and third patches are only for HEAD, while
the documentation hole with SET TABLESPACE should go down to v10.
Backpatching the tests of ALTER MATERIALIZED VIEW ALL IN TABLESPACE
would not hurt, either, as there is zero coverage for that now.
--
Michael
On Fri, Mar 18, 2022 at 03:13:05PM +0900, Michael Paquier wrote:
Thanks. This looks rather sane to me. I'll split things into 3
commits in total, as of the psql completion, SET TABLESPACE and SET
ACCESS METHOD. The first and third patches are only for HEAD, while
the documentation hole with SET TABLESPACE should go down to v10.
Backpatching the tests of ALTER MATERIALIZED VIEW ALL IN TABLESPACE
would not hurt, either, as there is zero coverage for that now.
I have applied the set, after splitting things mostly as mentioned
upthread:
- The doc change for SET TABLESPACE on ALTER MATVIEW has been
backpatched.
- The regression tests for SET TABLESPACE have been applied on HEAD,
as these are independent of the rest, good on their own.
- All the remaining parts for SET ACCESS METHOD (psql tab completion,
tests and docs) have been merged together on HEAD. I could not
understand why the completion done after SET ACCESS METHOD was not
grouped with the other parts for ALTER MATVIEW, so I have moved the
new entry there, and I have added a test checking after an error for
multiple subcommands, while on it.
Thanks, Nagata-san!
--
Michael
On Sat, 19 Mar 2022 19:31:59 +0900
Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Mar 18, 2022 at 03:13:05PM +0900, Michael Paquier wrote:
Thanks. This looks rather sane to me. I'll split things into 3
commits in total, as of the psql completion, SET TABLESPACE and SET
ACCESS METHOD. The first and third patches are only for HEAD, while
the documentation hole with SET TABLESPACE should go down to v10.
Backpatching the tests of ALTER MATERIALIZED VIEW ALL IN TABLESPACE
would not hurt, either, as there is zero coverage for that now.I have applied the set, after splitting things mostly as mentioned
upthread:
- The doc change for SET TABLESPACE on ALTER MATVIEW has been
backpatched.
- The regression tests for SET TABLESPACE have been applied on HEAD,
as these are independent of the rest, good on their own.
- All the remaining parts for SET ACCESS METHOD (psql tab completion,
tests and docs) have been merged together on HEAD. I could not
understand why the completion done after SET ACCESS METHOD was not
grouped with the other parts for ALTER MATVIEW, so I have moved the
new entry there, and I have added a test checking after an error for
multiple subcommands, while on it.Thanks, Nagata-san!
Thank you!
--
Yugo NAGATA <nagata@sraoss.co.jp>