Re: Allow CREATE OR REPLACE VIEW to rename the columns

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: btfujiitkp <btfujiitkp(at)oss(dot)nttdata(dot)com>
Cc: Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Allow CREATE OR REPLACE VIEW to rename the columns
Date: 2019-11-14 14:14:22
Message-ID: CAHGQGwFgR19uiYq+ZWoDsdsSYS0Z9s9pVdG5JFCNTcsuS5cDFA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 6, 2019 at 4:14 PM btfujiitkp <btfujiitkp(at)oss(dot)nttdata(dot)com> wrote:
>
> 2019-10-31 21:01 に Fujii Masao さんは書きました:
> > On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
> > wrote:
> >>
> >>
> >>
> >> On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
> >> wrote:
> >>>
> >>> On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>> >
> >>> > Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> >>> > > Currently CREATE OR REPLACE VIEW command fails if the column names
> >>> > > are changed.
> >>> >
> >>> > That is, I believe, intentional. It's an effective aid to catching
> >>> > mistakes in view redefinitions, such as misaligning the new set of
> >>> > columns relative to the old. That's particularly important given
> >>> > that we allow you to add columns during CREATE OR REPLACE VIEW.
> >>> > Consider the oversimplified case where you start with
> >>> >
> >>> > CREATE VIEW v AS SELECT 1 AS x, 2 AS y;
> >>> >
> >>> > and you want to add a column z, and you get sloppy and write
> >>> >
> >>> > CREATE OR REPLACE VIEW v AS SELECT 1 AS x, 3 AS z, 2 AS y;
> >>> >
> >>> > If we did not throw an error on this, references that formerly
> >>> > pointed to column y would now point to z (as that's still attnum 2),
> >>> > which is highly unlikely to be what you wanted.
> >>>
> >>> This example makes me wonder if the addtion of column by
> >>> CREATE OR REPLACE VIEW also has the same (or even worse) issue.
> >>> That is, it may increase the oppotunity for users' mistake.
> >>> I'm thinking the case where users mistakenly added new column
> >>> into the view when replacing the view definition. This mistake can
> >>> happen because CREATE OR REPLACE VIEW allows new column to
> >>> be added. But what's the worse is that, currently there is no way to
> >>> drop the column from the view, except recreation of the view.
> >>> Neither CREATE OR REPLACE VIEW nor ALTER TABLE support
> >>> the drop of the column from the view. So, to fix the mistake,
> >>> users would need to drop the view itself and recreate it. If there
> >>> are
> >>> some objects depending the view, they also might need to be
> >>> recreated.
> >>> This looks not good. Since the feature has been supported,
> >>> it's too late to say that, though...
> >>>
> >>> At least, the support for ALTER VIEW DROP COLUMN might be
> >>> necessary to alleviate that situation.
> >>>
> >>
> >> - Is this intentional not implemented the "RENAME COLUMN" statement
> >> for
> >> VIEW because it is implemented for Materialized View?
> >
> > Not sure that, but Tom's suggestion to support ALTER VIEW RENAME COLUMN
> > sounds reasonable whether we support the rename of columns when
> > replacing
> > the view definition, or not. Attached is the patch that adds support
> > for
> > ALTER VIEW RENAME COLUMN command.
> >
> >> I have made just a similar
> >> change to view and it works.
> >
> > Yeah, ISTM that we made the same patch at the same time. You changed
> > gram.y,
> > but I added the following changes additionally.
> >
> > - Update the doc
> > - Add HINT message emit when CRAETE OR REPLACE VIEW fails to rename the
> > columns
> > - Update tab-complete.c
>
>
> I review your patch, and then I found that tab complete of "alter
> materialized view" is also not enough.
> So, I made a small patch referencing your patch.

Good catch! The patch basically looks good to me.
But I think that "ALTER MATERIALIZED VIEW xxx <TAB>" should output also
DEPENDS ON EXTENSION, SET TABLESPACE, CLUSTER ON and RESET.
So I added such tab-completes to your patch. Patch attached.

Barring any objection, I'm thinking to commit this patch.

Regards,

--
Fujii Masao

Attachment Content-Type Size
alter_materialized_view_v2.patch application/octet-stream 1.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2019-11-14 14:22:01 Re: could not stat promote trigger file leads to shutdown
Previous Message Peter Eisentraut 2019-11-14 13:58:32 could not stat promote trigger file leads to shutdown