Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jLhTy-0007oX-1a for pgadmin-hackers@arkaria.postgresql.org; Tue, 07 Apr 2020 06:13:10 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1jLhTw-0000xq-Pw for pgadmin-hackers@arkaria.postgresql.org; Tue, 07 Apr 2020 06:13:08 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jLhTw-0000xj-EV for pgadmin-hackers@lists.postgresql.org; Tue, 07 Apr 2020 06:13:08 +0000 Received: from mail-il1-x131.google.com ([2607:f8b0:4864:20::131]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jLhTq-0001nW-9A for pgadmin-hackers@postgresql.org; Tue, 07 Apr 2020 06:13:07 +0000 Received: by mail-il1-x131.google.com with SMTP id j69so2034424ila.11 for ; Mon, 06 Apr 2020 23:13:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=F8RBxF0qc2awIJy7VZYHwZ35LmUzkKb8iE3h1Yu3ot8=; b=bSpsDVudYDoCspcOxuZZ5GyHMHIfl3SxLo6cpMz3A4g6Br9Tuh6TnKN01EnugokbQ4 L85j5SFJizFOstn+DYYdHATxvLoYqwZimOTiLNIGYhD3TRlm8V6xmrdii0BDY3Y1+evY ZD62S0rSidJOe19tbqGGN36iqPMpYAJkkkKKU6lSo+vna/Q6lGHttVRAGo19G6TX8nLw 2u/rmjLHNDOKb0iSIcKSXk3X87BAna0JoOdWFuZ5R4tZV8385TbtaHiA0dkWEZyVW1u5 OLUBKfDVdZ7uiBO8ezZTogTCWictnFRRqpYU4jyksZtF25gMDWJaC4JblnyrOfs/WYJR eBhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=F8RBxF0qc2awIJy7VZYHwZ35LmUzkKb8iE3h1Yu3ot8=; b=iweZVN6hmQm3dYxGhNNnQBP7pQWJ4KcX3hRl00I8KKUhJmygHNs83f8cjsno7liKRC CVisz72LObaFngq01TyJcPxil27QcmuwAkZgmTYLpqq6cQntZ3PYLLenCHeR7XctOEC4 +in8+rnYS7dor5ugZFqA2UGsGK1PHPGK6pm8SLxziBMYPrUpA9KfvULl4yqOiHy96pG+ GknSqGHAmMONNGAqY+//+XwYByyNghpv5rYUL5+TcC6PrUllx7wx8pgqF3tAFxd2zdBx PcmcSvyzTUVxv5FGLFvq4GyCBmTE8QWU9dBZ6dOWLeYLw7u1J9+XW47wv4vBNGuZiTX7 lavA== X-Gm-Message-State: AGi0PuaBVxNwBmYiLVNn7TFAWhAqU+1QirBeqtGig3ooCrSO8ngLaDxZ 6V8n8HYPtT1cOvvH3J4P2wJZAJ3zJDh7iKsCLqmJTWy/6wcILckGw13SarpOEVL3mRoA/aV3eFx 4bIw3CwIbqyAGzZQwSto69tsgW7hV6ZGa/D/M4/1UL33CUFp+8+8bEFc+Bkfpg30c8RDWxCe18d E24M7lPkO0zS8BkjHSy7Cfn2oz+XyfG+P7USC8hHvFGbjJGm+fGrY= X-Google-Smtp-Source: APiQypJHEWIDgjIpAZWvlw8L3xAR8R5I0hDw62MHAaNpWWNySN30gDIsxtQL19i106Es+2FE9sl3noQupmUCkj5vUFA= X-Received: by 2002:a92:4986:: with SMTP id k6mr774162ilg.252.1586239979654; Mon, 06 Apr 2020 23:12:59 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Tue, 7 Apr 2020 11:42:48 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch]: RM 5053 - Getting an error while changing the columns in the existing view To: Khushboo Vashi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="00000000000084296c05a2ad4690" X-CLOUD-SEC-AV-Info: edb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000084296c05a2ad4690 Content-Type: text/plain; charset="UTF-8" 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.vashi@enterprisedb.com> wrote: > Hi Akshay, > > Please find the attached updated patch. > > On Tue, Mar 24, 2020 at 2:47 PM Akshay Joshi < > akshay.joshi@enterprisedb.com> wrote: > >> Hi Khushboo >> >> On Tue, Mar 24, 2020 at 1:47 PM Khushboo Vashi < >> khushboo.vashi@enterprisedb.com> wrote: >> >>> Hi Akshay, >>> >>> On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi < >>> akshay.joshi@enterprisedb.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.vashi@enterprisedb.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* --00000000000084296c05a2ad4690 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Khushboo

The warning message is= not showing=C2=A0up. Please fix and resend the patch.

On Tue, Apr 7, = 2020 at 10:00 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi Akshay,=

Please find the attached updated patch.

=
On Tue, Ma= r 24, 2020 at 2:47 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:=
Hi=C2=A0Khushboo

On Tue, Mar 24, 2020 at 1:47 PM Khushboo= Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi Ak= shay,

On Tue, Jan 14, 2020 at 11:47 AM Akshay Joshi <akshay.joshi@enterprisedb.co= m> wrote:
Hi=C2=A0Khushboo

Following are the rev= iew comments:
  • Fix the PEP8 issue.
  • Drop query shou= ld be part of the jinja template for consistency. Currently, it is added th= rough the python file.
The Delete qu= ery is already in the template file, I have just reused the delete call and= merged the SQL queries in the python file.=C2=A0
  • 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'.=C2=A0
  • =
I have tested but=C2=A0 couldn't rep= roduce this issue.=C2=A0 Can you please let me know the proper use case?

=C2=A0 =C2=A0Create a view = with 'SELECT 1;' as code. Then change the code to 'SELECT 1234;= ' and click on the Save button.=C2=A0
=C2=A0 =C2=A0Warning po= pup is displayed "Changing the columns in a view....". Click on t= he 'Yes' button and check the OID of the view. You will get the sam= e OID, it means view is not recreated.
=C2=A0 =C2=A0=C2=A0
<= /div>
I can reproduce this issue with the given SQL = but the problem is as per the PostgreSQL documentation, (Ref:=C2=A0https://www.postgresql.org/docs/12/sql-createview.html)

=
&qu= ot;CREATE OR REPLACE VIEW=C2=A0is similar, but if a view of the = same name already exists, it is replaced. The new query must generate the s= ame columns that were generated by the existing view query (that is, the sa= me 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 ris= e to the output columns may be completely different."

So, I put a check on the columns and if the column is chang= ed, 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.
=C2=A0
=C2=A0 =C2=A0 = I have observed below error in the browser while changing the code:=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 view.js:241 Uncaught Typ= eError: Cannot read property 'replace' of undefined
=C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 at child.onChange (view.js:241)
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 at HTMLDivElement.dispatch (jquery.j= s:5237)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 at HTMLDivElement.elem= Data.handle (jquery.js:5044)

Fixed.=C2=A0

Thanks,
Khushboo

Thanks,
Khushboo
=C2=A0

On Tue, Jan 14, 2020 at 10:27 AM Khushboo Vashi <khushboo.vashi@enterprisedb.= com> wrote:
Hi,

Please find the attached patch f= or RM #5053 -=C2=A0Getting an error while changing the columns in the exist= ing view.

PostgreSQL doesn't allow to change t= he 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
<= font color=3D"#3333FF">Sr. Software Architect=
<= font color=3D"#000000" face=3D"arial, sans-serif">EnterpriseDB Software = India Private Limited
Mobile: +91 976-788-8246=


--
Thanks & Regards
Akshay Joshi
<= font color=3D"#3333FF">Sr. Software Architect=
<= font color=3D"#000000" face=3D"arial, sans-serif">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
--00000000000084296c05a2ad4690--