Re: Allow CREATE OR REPLACE VIEW to rename the columns

On Thu, Oct 31, 2019 at 5:28 PM Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> wrote:

>
>
> On Thu, Oct 31, 2019 at 5:11 PM Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> wrote:
>
>>
>>
>> On Thu, Oct 31, 2019 at 5:01 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
>> wrote:
>>
>>> 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
>>> - 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?

>
>>
>>> Regards,
>>>
>>> --
>>> Fujii Masao
>>>
>>
>>
>> --
>> Ibrar Ahmed
>>
>
>
> --
> Ibrar Ahmed
>

--
Ibrar Ahmed