Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1etzqd-0007GC-0k for pgadmin-hackers@arkaria.postgresql.org; Thu, 08 Mar 2018 18:00:59 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1etzqb-0004Jt-Tu for pgadmin-hackers@arkaria.postgresql.org; Thu, 08 Mar 2018 18:00:57 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1etzqb-0004Jj-Ht for pgadmin-hackers@lists.postgresql.org; Thu, 08 Mar 2018 18:00:57 +0000 Received: from mail-oi0-x231.google.com ([2607:f8b0:4003:c06::231]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1etzqW-0004ew-JI for pgadmin-hackers@postgresql.org; Thu, 08 Mar 2018 18:00:57 +0000 Received: by mail-oi0-x231.google.com with SMTP id f186so5015208oig.4 for ; Thu, 08 Mar 2018 10:00:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=uIWNNKpPtzVZSB1QmbY6SaZQwnS3zrpgmUUIwfZLSB4=; b=Q3g91ivJTjZfJeAdi6xnD7F9GB7ma4DAJx7tx+i0NDQfbJFU8/NF54C9yTchhanZqW gpO7AXBML2ebc3kTSzzqxXeN3dKkjSMGEYrKvoMK8Jtbt3y3yC01CQ6D0yJu114/rXjn qUIswuENjrSsGkCWHQPWFlBFjLF9KyeRDsS9yQhAkSijFbzr1q6w48Q9SxpxAB5nN3J6 0AhAeCsFX+X70d+wsIHA9pFTmUaOyYcH9fSYWT2VyW5y9UUYRM+Rummn/UR3BqJb0r8U /HoUJ1fiUV+P2UQWApsPAW323RzhEhIascAC3JY27tNjvmLOjQZV4lCtRGjq4zHcHU5G fAeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=uIWNNKpPtzVZSB1QmbY6SaZQwnS3zrpgmUUIwfZLSB4=; b=IXPP/XKkZlEWISu35w+D+PY4PrmzxpJkpZZJWnN3/uGsHoFbAyNmy5P9EPhLqXBbXG of2oqR9Y3F6wS0M6N+o9K1aZdYtdjUynD5NrFk40nCIjKtCQTgCeutlAb9kFghgXK4CR ZwP2KKSI4V3dr/r6/p0ZLbc+y2w8edEnEUGHaNscta0I8gWcKQC5IpuQz1XUexXEHlhe 9y1Mo82aPdCcNuO8RnRboSWnEyvkY+9Knm4Fv+U89qZfeKz0UAz3+KDMaHQfDAtYTY8C NnigeKu2PI8S0xJM9RrlxQJnM/1q6RsEZ2zjJw7plQi+6G2/SxAl3nY12IN6FJeMEODw WN4g== X-Gm-Message-State: AElRT7H7MFQFfA5drbCyL4d2GZEJ1CJRDIknXNPB28D+2jeBDuXAOt86 aNf80rdtT+iKQqzFm+AjjrvxN/93kdVFDYUp7pKKEg== X-Google-Smtp-Source: AG47ELsCiYz01VP1WSroyF+tAvbt+QFuit4NWLjTAoeOWw9UPjoj7ZCGH5O9oqIFD4r5TkNxmItP1Iz+VbcGBmx06ww= X-Received: by 10.202.63.85 with SMTP id m82mr17896270oia.64.1520532048527; Thu, 08 Mar 2018 10:00:48 -0800 (PST) MIME-Version: 1.0 Received: by 10.74.8.150 with HTTP; Thu, 8 Mar 2018 10:00:28 -0800 (PST) In-Reply-To: References: From: Murtuza Zabuawala Date: Thu, 8 Mar 2018 23:30:28 +0530 Message-ID: Subject: Re: [pgAdmin4][RM#2989] To fix the issue in Table node To: Joao De Almeida Pereira Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary="001a113dd4929f62e10566ea7457" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a113dd4929f62e10566ea7457 Content-Type: text/plain; charset="UTF-8" Thank you Joao Regards, Murtuza On Thu, Mar 8, 2018 at 10:19 PM, Joao De Almeida Pereira < jdealmeidapereira@pivotal.io> wrote: > Hello Murtuza/Dave, > > Nice splitting of some of the functionality into functions, removing some > of the complexity of the initial function. Good job. > > I made some changes because the linter was failing and also changed some > variable names. > These changes pass our CI and the linter. > > Thanks > Joao > > On Thu, Mar 8, 2018 at 8:13 AM Murtuza Zabuawala enterprisedb.com> wrote: > >> Hi Dave, >> >> Please find updated patch. >> >> -- >> Regards, >> Murtuza Zabuawala >> EnterpriseDB: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> >> On Thu, Mar 8, 2018 at 6:10 PM, Dave Page wrote: >> >>> Can you rebase this please? >>> >>> Thanks. >>> >>> On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala >> enterprisedb.com> wrote: >>> >>>> Hi Dave, >>>> >>>> Please find updated patch & updated test case to cover that as well. >>>> >>>> >>>> >>>> -- >>>> Regards, >>>> Murtuza Zabuawala >>>> EnterpriseDB: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>>> >>>> On Wed, Mar 7, 2018 at 9:59 PM, Dave Page wrote: >>>> >>>>> Hi >>>>> >>>>> On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala >>>> enterprisedb.com> wrote: >>>>> >>>>>> Hi Dave, >>>>>> >>>>>> PFA updated patch. >>>>>> >>>>>> >>>>> Using your example on the ticket, I added a "character varying (32)" >>>>> column with NOT NULL to the table. When I then edit the column, and turn >>>>> off NOT NULL (making no other changes), the SQL generated is: >>>>> >>>>> ALTER TABLE public.test_drop >>>>> ALTER COLUMN col2 TYPE character varying (32) COLLATE >>>>> pg_catalog."default"; >>>>> ALTER TABLE public.test_drop >>>>> ALTER COLUMN col2 DROP NOT NULL; >>>>> >>>>> I didn't see that when turning off NOT NULL for col1. >>>>> >>>>> -- >>>>> Dave Page >>>>> Blog: http://pgsnake.blogspot.com >>>>> Twitter: @pgsnake >>>>> >>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>> The Enterprise PostgreSQL Company >>>>> >>>> >>>> >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> --001a113dd4929f62e10566ea7457 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thank you Joao
Regards,
Murtuza=C2=A0


On Thu, Mar 8, 2018 at 10:19 PM,= Joao De Almeida Pereira <jdealmeidapereira@pivotal.io><= /span> wrote:
Hello Murt= uza/Dave,

Nice splitting of some of the functionality in= to functions, removing some of the complexity of the initial function. Good= job.

I made some changes because the linter was f= ailing and also changed some variable names.
These changes pass o= ur CI and the linter.

Thanks
Joao
<= br>
On Thu, Mar 8, 2018 at 8:13 = AM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote= :
Hi Dave,

Please find updated patch.
<= /div>

--
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The Enterprise PostgreSQL Company<= /font>


On Thu, Mar= 8, 2018 at 6:10 PM, Dave Page <dpage@pgadmin.org> wrote:
Can you rebase this please= ?

Thanks.

On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wro= te:
Hi Dave,

Please find updated patch & updated t= est case to cover that as well.



--
Regards,Mur= tuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Wed, Mar 7, 2018 at 9:59 PM, Dave Page <= dpage@pgadmin.org> wrote:
<= div dir=3D"ltr">Hi

On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala &l= t;m= urtuza.zabuawala@enterprisedb.com> wrote:
Hi Dave,

PFA updated patch.


Using your example on the ticket, I added a "character varying (32)= " column with NOT NULL to the table. When I then edit the column, and = turn off NOT NULL (making no other changes), the SQL generated is:

ALTER TABLE public.test_drop
=C2=A0 =C2=A0 = ALTER COLUMN col2 TYPE character varying (32) COLLATE pg_catalog."defa= ult";
ALTER TABLE public.test_drop
=C2=A0 =C2=A0 A= LTER COLUMN col2 DROP NOT NULL;

I didn't= see that when turning off NOT NULL for col1.

--
Dave Page
Blog: http://pgsnake.blogspot.comTwitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterpris= e PostgreSQL Company




-- =
Dave Page
B= log: http://pgsna= ke.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com=
The Enterprise PostgreSQL Company
--001a113dd4929f62e10566ea7457--