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

From: Khushboo Vashi <[email-masked]>
Date: Tue, 14 Jan 2020 10:27:31 +0530

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

---

From: Akshay Joshi <[email-masked]>
Date: Tue, 14 Jan 2020 11:47:04 +0530

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

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*

---

From: Khushboo Vashi <[email-masked]>
Date: Tue, 24 Mar 2020 13:47:37 +0530

Hi Akshay,

On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <[email-masked]>
wrote:

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

> I have tested but  couldn't reproduce this issue.  Can you please let me
know the proper use case?

Thanks,
Khushboo

---

From: Akshay Joshi <[email-masked]>
Date: Tue, 24 Mar 2020 14:47:38 +0530

Hi Khushboo

On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi <
[email-masked]> wrote:

> Hi Akshay,
>
> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <
> [email-masked]> wrote:
>
>> Hi Khushboo
>>

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

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

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*

---

From: Khushboo Vashi <[email-masked]>
Date: Tue, 7 Apr 2020 10:00:18 +0530

Hi Akshay,

Please find the attached updated patch.

On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <[email-masked]>
wrote:

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

> Fixed.

Thanks,
Khushboo

---

From: Akshay Joshi <[email-masked]>
Date: Tue, 7 Apr 2020 11:42:48 +0530

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 <
[email-masked]> wrote:

> On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi <
>>>> 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.
>>>
>>>>
>>>> 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.
> I can reproduce this issue with the given SQL but the problem is as per
> the PostgreSQL documentation, (Ref:
> "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."
>
>>             view.js:241 Uncaught TypeError: Cannot read property
>> 'replace' of undefined

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*

---

From: Khushboo Vashi <[email-masked]>
Date: Wed, 8 Apr 2020 10:58:29 +0530

Hi Akshay,

Thanks,
Khushboo

On Tue, Apr 7, 2020 at 11:43 AM Akshay Joshi <[email-masked]>
wrote:

>>>    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.
>>>
>>>
>> So, I put a check on the columns and if the column is changed, the
>> message will popup.
>>

---

From: Akshay Joshi <[email-masked]>
Date: Wed, 8 Apr 2020 11:42:22 +0530

Thanks, patch applied.

On Wed, Apr 8, 2020 at 10:58 AM Khushboo Vashi <
[email-masked]> wrote:

> On Tue, Apr 7, 2020 at 11:43 AM Akshay Joshi <
>>>>>>    - Drop query should be part of the jinja template for
>>>>>>    consistency. Currently, it is added through the python file.
>>>>>>
>>> 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

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*

---

From: Khushboo Vashi <[email-masked]>
Date: Mon, 13 Apr 2020 14:29:40 +0530

Hi,

Please find the attached patch to fix the test cases due to this patch.
Also, this functionality will not be applicable on EPAS server as we can
change the view definition without dropping it.

Thanks,
Khushboo

On Wed, Apr 8, 2020 at 11:42 AM Akshay Joshi <[email-masked]>
wrote:

>>>>>>>    - 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?
>>>>>>
>>>>>
>>>>>>>> Please find the attached patch for RM #5053 - Getting an error
>>>>>>>> while changing the columns in the existing view.
>>>>>>>>

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 13 Apr 2020 20:25:27 +0530

Thanks, patch applied.

On Mon, Apr 13, 2020 at 2:29 PM Khushboo Vashi <
[email-masked]> wrote:

> On Wed, Apr 8, 2020 at 11:42 AM Akshay Joshi <
>>>>>>>> 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.
>>>>>>>
>>>>>>>>
>>>>> I can reproduce this issue with the given SQL but the problem is as
>>>>> per the PostgreSQL documentation, (Ref:

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*