Thread: 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>
Attachment
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
Attachment
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>
Attachment
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
Attachment
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
Attachment
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>