Horton

Thread

Commits

  1. Improve handling of SET ACCESS METHOD for ALTER MATERIALIZED VIEW

  2. Add regression tests for ALTER MATERIALIZED VIEW with tablespaces

  3. doc: Mention SET TABLESPACE clause for ALTER MATERIALIZED VIEW

  4. Add support for SET ACCESS METHOD in ALTER TABLE

  1. Tab completion for ALTER MATERIALIZED VIEW ... SET ACCESS METHOD

    Yugo Nagata <nagata@sraoss.co.jp> — 2022-03-16T04:33:37Z

    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>

  2. Re: Tab completion for ALTER MATERIALIZED VIEW ... SET ACCESS METHOD

    Michael Paquier <michael@paquier.xyz> — 2022-03-16T07:18:09Z

    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

  3. Re: Tab completion for ALTER MATERIALIZED VIEW ... SET ACCESS METHOD

    Yugo Nagata <nagata@sraoss.co.jp> — 2022-03-18T01:27:41Z

    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>

  4. Re: Tab completion for ALTER MATERIALIZED VIEW ... SET ACCESS METHOD

    Michael Paquier <michael@paquier.xyz> — 2022-03-18T06:13:05Z

    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

  5. Re: Tab completion for ALTER MATERIALIZED VIEW ... SET ACCESS METHOD

    Michael Paquier <michael@paquier.xyz> — 2022-03-19T10:31:59Z

    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

  6. Re: Tab completion for ALTER MATERIALIZED VIEW ... SET ACCESS METHOD

    Yugo Nagata <nagata@sraoss.co.jp> — 2022-03-22T08:53:17Z

    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>