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 Page <dp...@pgadmin.org> wrote:
>
>> 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 <dp...@pgadmin.org> 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
>>
>
>

Attachment: RM_2989_version3.diff
Description: Binary data

Reply via email to