Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view
Date: 2020-04-08 05:28:29
Message-ID: CAFOhELef+GJfcNfVK1Ew5e5gN1mhMOtDZsYL2pfj-4WzBrxQ-Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Akshay,

Please find the attached updated patch.

Thanks,
Khushboo

On Tue, Apr 7, 2020 at 11:43 AM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Khushboo
>
> The warning message is not showing up. Please fix and resend the patch.
>
> On Tue, Apr 7, 2020 at 10:00 AM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi Akshay,
>>
>> Please find the attached updated patch.
>>
>> On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Khushboo
>>>
>>> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Khushboo
>>>>>
>>>>> Following are the review comments:
>>>>>
>>>>> - Fix the PEP8 issue.
>>>>> - Drop query should be part of the jinja template for consistency.
>>>>> Currently, it is added through the python file.
>>>>>
>>>>> The Delete query is already in the template file, I have just reused
>>>> the delete call and merged the SQL queries in the python file.
>>>>
>>>>>
>>>>> - Any changes in the view code should not warn the user "Changing
>>>>> the columns in a view requires dropping...." and we should not drop the
>>>>> view. For example, I have only change the WHERE clause or added 'ORDER BY'.
>>>>>
>>>>> I have tested but couldn't reproduce this issue. Can you please let
>>>> me know the proper use case?
>>>>
>>>
>>> Create a view with 'SELECT 1;' as code. Then change the code to
>>> 'SELECT 1234;' and click on the Save button.
>>> Warning popup is displayed "Changing the columns in a view....".
>>> Click on the 'Yes' button and check the OID of the view. You will get the
>>> same OID, it means view is not recreated.
>>>
>>>
>> I can reproduce this issue with the given SQL but the problem is as per
>> the PostgreSQL documentation, (Ref:
>> https://www.postgresql.org/docs/12/sql-createview.html)
>>
>> "CREATE OR REPLACE VIEW is similar, but if a view of the same name
>> already exists, it is replaced. The new query must generate the same
>> columns that were generated by the existing view query (that is, the same
>> column names in the same order and with the same data types), but it may
>> add additional columns to the end of the list. The calculations giving rise
>> to the output columns may be completely different."
>>
>> So, I put a check on the columns and if the column is changed, the
>> message will popup.
>>
>> In case of the example given by you, the column name is not changed as if
>> you don't give the column name it will be default and I think view would
>> have the column names properly.
>>
>>
>>> I have observed below error in the browser while changing the code:
>>> view.js:241 Uncaught TypeError: Cannot read property
>>> 'replace' of undefined
>>> at child.onChange (view.js:241)
>>> at HTMLDivElement.dispatch (jquery.js:5237)
>>> at HTMLDivElement.elemData.handle (jquery.js:5044)
>>>
>>> Fixed.
>>
>> Thanks,
>> Khushboo
>>
>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>>>
>>>>>
>>>>
>>>>> On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please find the attached patch for RM #5053 - Getting an error while
>>>>>> changing the columns in the existing view.
>>>>>>
>>>>>> PostgreSQL doesn't allow to change the view columns. So, while
>>>>>> performing this task the existing view should be dropped first and then
>>>>>> recreate it and also user will get a warning first.
>>>>>>
>>>>>> Thanks,
>>>>>> Khushboo
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards*
>>>>> *Akshay Joshi*
>>>>>
>>>>> *Sr. Software Architect*
>>>>> *EnterpriseDB Software India Private Limited*
>>>>> *Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>

Attachment Content-Type Size
RM_5053_v2.patch application/octet-stream 8.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2020-04-08 05:59:19 pgAdmin 4 commit: Fixes an issue where pgAdmin4 GUI does not display pr
Previous Message Ken Benson 2020-04-07 19:06:16 RE: Proposal: Drop support for Internet Explorer