Thread: Allow CREATE OR REPLACE VIEW to rename the columns

Hi,

Currently CREATE OR REPLACE VIEW command fails if the column names
are changed. For example,

    =# CREATE VIEW test AS SELECT 0 AS a;
    =# CREATE OR REPLACE VIEW test AS SELECT 0 AS x;
    ERROR:  cannot change name of view column "a" to "x"

I'd like to propose the attached patch that allows CREATE OR REPLACE VIEW
to rename the view columns. Thought?

Regards,

-- 
Fujii Masao

Fujii Masao <masao.fujii@gmail.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.

The right way to handle a column rename in a view is to do a separate
ALTER VIEW RENAME COLUMN, making it totally clear what you intend to
happen.  (Right now, we make you spell that "ALTER TABLE RENAME COLUMN",
but it'd be reasonable to add that syntax to ALTER VIEW too.)  I don't
think this functionality should be folded into redefinition of the content
of the view.  It'd add more opportunity for mistakes than anything else.

            regards, tom lane

On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Fujii Masao <masao.fujii@gmail.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.

Regards,

-- 
Fujii Masao

On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
>
>
>
> On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> >
>> > Fujii Masao <masao.fujii@gmail.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
- Add regression test

One issue I've not addressed yet is about the command tag of
"ALTER VIEW RENAME COLUMN". Currently "ALTER TABLE" is returned as the tag
like ALTER MATERIALIZED VIEW RENAME COLUMN, but ISTM that "ALTER VIEW"
is better. I started the discussion about the command tag of
"ALTER MATERIALIZED VIEW" at another thread. I will update the patch according
to the result of that discussion.
https://postgr.es/m/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com

Regards,

-- 
Fujii Masao

Fujii Masao <masao.fujii@gmail.com> writes:
> On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Fujii Masao <masao.fujii@gmail.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.  [example]

> 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.

The idea in CREATE OR REPLACE VIEW is to allow addition of new
columns at the end, the same as you can do with tables.  Checking
the column name matchups is a way to ensure that you actually do
add at the end, rather than insert, which wouldn't act as you
expect.  Admittedly it's only heuristic.

We could, perhaps, have insisted that adding a column also requires
special syntax, but we didn't.  Consider for example a case where
the new column needs to come from an additionally joined table;
then you have to be able to edit the underlying view definition along
with adding the column.  So that seems like kind of a pain in the
neck to insist on.

> But what's the worse is that, currently there is no way to
> drop the column from the view, except recreation of the view.

I think this has been discussed, as well.  It's not necessarily
simple to drop a view output column.  For example, if the view
uses SELECT DISTINCT, removing an output column would have
semantic effects on the set of rows that can be returned, since
distinct-ness would now mean something else than it did before.

It's conceivable that we could enumerate all such effects and
then allow DROP COLUMN (probably replacing the output column
with a null constant) if none of them apply, but I can't get
terribly excited about it.  The field demand for such a feature
has not been high.  I'd be a bit worried about bugs arising
from failures to check attisdropped for views, too; so that
the cost of getting this working might be greater than it seems
on the surface.

            regards, tom lane

On Thu, Oct 31, 2019 at 10:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Fujii Masao <masao.fujii@gmail.com> writes:
> > On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Fujii Masao <masao.fujii@gmail.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.  [example]
>
> > 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.
>
> The idea in CREATE OR REPLACE VIEW is to allow addition of new
> columns at the end, the same as you can do with tables.  Checking
> the column name matchups is a way to ensure that you actually do
> add at the end, rather than insert, which wouldn't act as you
> expect.  Admittedly it's only heuristic.
>
> We could, perhaps, have insisted that adding a column also requires
> special syntax, but we didn't.  Consider for example a case where
> the new column needs to come from an additionally joined table;
> then you have to be able to edit the underlying view definition along
> with adding the column.  So that seems like kind of a pain in the
> neck to insist on.
>
> > But what's the worse is that, currently there is no way to
> > drop the column from the view, except recreation of the view.
>
> I think this has been discussed, as well.  It's not necessarily
> simple to drop a view output column.  For example, if the view
> uses SELECT DISTINCT, removing an output column would have
> semantic effects on the set of rows that can be returned, since
> distinct-ness would now mean something else than it did before.
>
> It's conceivable that we could enumerate all such effects and
> then allow DROP COLUMN (probably replacing the output column
> with a null constant) if none of them apply, but I can't get
> terribly excited about it.  The field demand for such a feature
> has not been high.  I'd be a bit worried about bugs arising
> from failures to check attisdropped for views, too; so that
> the cost of getting this working might be greater than it seems
> on the surface.

Thanks for the explanation! Understood.

Regards,

-- 
Fujii Masao

2019-10-31 21:01 に Fujii Masao さんは書きました:
> On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> 
> wrote:
>> 
>> 
>> 
>> On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com> 
>> wrote:
>>> 
>>> On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> >
>>> > Fujii Masao <masao.fujii@gmail.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.

Regards,

On Thu, Oct 31, 2019 at 9:34 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
>
>
>
> On Thu, Oct 31, 2019 at 5:28 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
>>
>>
>>
>> On Thu, Oct 31, 2019 at 5:11 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
>>>
>>>
>>>
>>> On Thu, Oct 31, 2019 at 5:01 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>>>>
>>>> On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
>>>> >
>>>> >
>>>> >
>>>> > On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>>>> >>
>>>> >> On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> >> >
>>>> >> > Fujii Masao <masao.fujii@gmail.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
>>>> - Add regression test
>>>>
>>>
>>> Oh, I just sent the patch to ask, good you do that in all the places.
>>>
>>>> One issue I've not addressed yet is about the command tag of
>>>> "ALTER VIEW RENAME COLUMN". Currently "ALTER TABLE" is returned as the tag
>>>> like ALTER MATERIALIZED VIEW RENAME COLUMN, but ISTM that "ALTER VIEW"
>>>> is better. I started the discussion about the command tag of
>>>> "ALTER MATERIALIZED VIEW" at another thread. I will update the patch according
>>>> to the result of that discussion.
>>>> https://postgr.es/m/CAHGQGwGUaC03FFdTFoHsCuDrrNvFvNVQ6xyd40==P25WvuBJjg@mail.gmail.com
>>>>
>>> Attached patch contain small change for ALTER MATERIALIZED VIEW.
>>>
>>
>> Hmm, my small change of "ALTER MATERIALIZED VIEW" does not work in some cases need more work on that.
>>
>
>
> The AlterObjectTypeCommandTag function just take one parameter, but to
> show  "ALTER MATERIALIZED VIEW" instead of ALTER TABLE we need to
> pass "relationType = OBJECT_MATVIEW" along with "renameType = OBJECT_COLUMN"
> and handle that in the function. The "AlterObjectTypeCommandTag" function has many
> calls. Do you think just for the command tag, we should do all this change?

Thanks for trying to address the issue!
As probably you've already noticed, the commit 979766c0af fixed the issue
thanks to your review. So we can review the patch that I posted.

Regards,

-- 
Fujii Masao

On Wed, Nov 6, 2019 at 4:14 PM btfujiitkp <btfujiitkp@oss.nttdata.com> wrote:
>
> 2019-10-31 21:01 に Fujii Masao さんは書きました:
> > On Thu, Oct 31, 2019 at 7:59 PM Ibrar Ahmed <ibrar.ahmad@gmail.com>
> > wrote:
> >>
> >>
> >>
> >> On Thu, Oct 31, 2019 at 12:32 PM Fujii Masao <masao.fujii@gmail.com>
> >> wrote:
> >>>
> >>> On Thu, Oct 31, 2019 at 1:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>> >
> >>> > Fujii Masao <masao.fujii@gmail.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

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

Build and All Test has passed .
Looks good to me .

Regards,

On Wed, Nov 20, 2019 at 1:11 PM btkimurayuzk
<btkimurayuzk@oss.nttdata.com> wrote:
>
> > Barring any objection, I'm thinking to commit this patch.
> >
> > Regards,
>
> Build and All Test has passed .
> Looks good to me .

Thanks for reviewing the patch!
I committed the following two patches.

- Allow ALTER VIEW command to rename the column in the view.
- Improve tab-completion for ALTER MATERIALIZED VIEW.

Regards,

-- 
Fujii Masao