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+10-1
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+66-2
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>