Re: [pgAdmin4][RM#2989] To fix the issue in Table node
Thanks, patch applied. On Thu, Mar 8, 2018 at 6:00 PM, Murtuza Zabuawala < murtuza.zabuaw...@enterprisedb.com> wrote: > Thank you Joao > > Regards, > Murtuza > > > On Thu, Mar 8, 2018 at 10:19 PM, Joao De Almeida Pereira < > jdealmeidapere...@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 < >> murtuza.zabuaw...@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 Pagewrote: >>> Can you rebase this please? Thanks. On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala < murtuza.zabuaw...@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 < >> murtuza.zabuaw...@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 >>> >>> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
Thank you Joao Regards, Murtuza On Thu, Mar 8, 2018 at 10:19 PM, Joao De Almeida Pereira < jdealmeidapere...@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 Zabuawalaenterprisedb.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 >>> >> >>
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
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 Pagewrote: > 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 < >>> murtuza.zabuaw...@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 > RM_2989_v2_rebase.diff Description: Binary data
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
Can you rebase this please? Thanks. On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala < murtuza.zabuaw...@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 Pagewrote: > >> Hi >> >> On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala < >> murtuza.zabuaw...@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
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
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 Pagewrote: > 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 > RM_2989_v2.diff Description: Binary data
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
Hi On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala < murtuza.zabuaw...@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
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
Hi Dave, PFA updated patch. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company On Wed, Mar 7, 2018 at 6:14 PM, Murtuza Zabuawala < murtuza.zabuaw...@enterprisedb.com> wrote: > > On Wed, Mar 7, 2018 at 6:12 PM, Dave Pagewrote: > >> Hi >> >> On Tue, Mar 6, 2018 at 4:06 PM, Joao De Almeida Pereira < >> jdealmeidapere...@pivotal.io> wrote: >> >>> Hi Murtuza, >>> >>> The code change works, and I passed the patches through our pipeline and >>> everything is green. >>> Personally I would love this bug fixes to have refactored the function >>> into smaller chunk and made it more readable so that the next time someone >>> need to check out a problem in the same area it is easier. I understand >>> that without a good test coverage it is hard to have confidence while >>> refactoring, but we need to start somewhere. >>> >> >> Are you planning to look into this Murtuza? >> > Yes Dave, I am working on another issue, I'll pick up after that. > > >> >> >>> >>> @Hackers >>> Here is a video that I saw some time ago about refactoring existing code >>> and code complexity that is very interesting >>> https://www.youtube.com/watch?v=8bZh5LMaSmE >>> In this video Sandi Metz does the Gilded Rose Kata in a talk in >>> RailsConf 2014, and with it tries to demonstrate that code can be >>> refactored and with that it make the code much more simpler. But the >>> journey is not always simple and the complexity will increase before it get >>> simpler. It is a good example of something that we can try with our code. >>> >> >> Nice! >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > RM_2989_v1.diff Description: Binary data
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
On Wed, Mar 7, 2018 at 6:12 PM, Dave Pagewrote: > Hi > > On Tue, Mar 6, 2018 at 4:06 PM, Joao De Almeida Pereira < > jdealmeidapere...@pivotal.io> wrote: > >> Hi Murtuza, >> >> The code change works, and I passed the patches through our pipeline and >> everything is green. >> Personally I would love this bug fixes to have refactored the function >> into smaller chunk and made it more readable so that the next time someone >> need to check out a problem in the same area it is easier. I understand >> that without a good test coverage it is hard to have confidence while >> refactoring, but we need to start somewhere. >> > > Are you planning to look into this Murtuza? > Yes Dave, I am working on another issue, I'll pick up after that. > > >> >> @Hackers >> Here is a video that I saw some time ago about refactoring existing code >> and code complexity that is very interesting >> https://www.youtube.com/watch?v=8bZh5LMaSmE >> In this video Sandi Metz does the Gilded Rose Kata in a talk in RailsConf >> 2014, and with it tries to demonstrate that code can be refactored and with >> that it make the code much more simpler. But the journey is not always >> simple and the complexity will increase before it get simpler. It is a good >> example of something that we can try with our code. >> > > Nice! > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
Hi On Tue, Mar 6, 2018 at 4:06 PM, Joao De Almeida Pereira < jdealmeidapere...@pivotal.io> wrote: > Hi Murtuza, > > The code change works, and I passed the patches through our pipeline and > everything is green. > Personally I would love this bug fixes to have refactored the function > into smaller chunk and made it more readable so that the next time someone > need to check out a problem in the same area it is easier. I understand > that without a good test coverage it is hard to have confidence while > refactoring, but we need to start somewhere. > Are you planning to look into this Murtuza? > > @Hackers > Here is a video that I saw some time ago about refactoring existing code > and code complexity that is very interesting > https://www.youtube.com/watch?v=8bZh5LMaSmE > In this video Sandi Metz does the Gilded Rose Kata in a talk in RailsConf > 2014, and with it tries to demonstrate that code can be refactored and with > that it make the code much more simpler. But the journey is not always > simple and the complexity will increase before it get simpler. It is a good > example of something that we can try with our code. > Nice! -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [pgAdmin4][RM#2989] To fix the issue in Table node
Hi Murtuza, The code change works, and I passed the patches through our pipeline and everything is green. Personally I would love this bug fixes to have refactored the function into smaller chunk and made it more readable so that the next time someone need to check out a problem in the same area it is easier. I understand that without a good test coverage it is hard to have confidence while refactoring, but we need to start somewhere. @Hackers Here is a video that I saw some time ago about refactoring existing code and code complexity that is very interesting https://www.youtube.com/watch?v=8bZh5LMaSmE In this video Sandi Metz does the Gilded Rose Kata in a talk in RailsConf 2014, and with it tries to demonstrate that code can be refactored and with that it make the code much more simpler. But the journey is not always simple and the complexity will increase before it get simpler. It is a good example of something that we can try with our code. Thanks Joao On Tue, Mar 6, 2018 at 4:23 AM Murtuza Zabuawala < murtuza.zabuaw...@enterprisedb.com> wrote: > Hi, > > PFA patch to fix the issue in Table node where wrong sql was generated > while altering column. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > >
[pgAdmin4][RM#2989] To fix the issue in Table node
Hi, PFA patch to fix the issue in Table node where wrong sql was generated while altering column. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company RM_2989.diff Description: Binary data fix_table_node_PEP8.diff Description: Binary data