Re: [pgAdmin4] [Patch]: Foreign Table Module

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4] [Patch]: Foreign Table Module
Date: 2016-05-14 17:06:10
Message-ID: CAKKotZS4xxwn9o=Pw_T1Sk+WP7b+gcmhjvgLJ+RFKnBQRTKX6Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

One last otherwise rest of functionality looks good.

- We are not able to drop/delete any constraints, not even empty blank row
once added.

Regards,
Murtuza

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, May 13, 2016 at 3:25 PM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi,
>
> Please find the attached updated patch for the foreign table module.
>
> Thanks,
> Khushboo
>
> On Fri, May 13, 2016 at 10:50 AM, Murtuza Zabuawala <
> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>
>> *1)* If we delete one inherited table and add another one
>> It generated drop columns sql for that inherited table which throws error,
>> for example "Test123"."TEST" has col1, col2 in it, once we remove table
>> see the wrong drop column sql.
>>
>> SQL:
>> ----
>> ALTER FOREIGN TABLE pem.asdas
>> DROP COLUMN col1;
>>
>> ALTER FOREIGN TABLE pem.asdas
>> DROP COLUMN col2;
>>
>> ALTER FOREIGN TABLE pem.asdas INHERIT pemdata.number_of_wal_files;
>>
>> ALTER FOREIGN TABLE pem.asdas NO INHERIT "Test123"."TEST";
>>
>> Expected:
>> ---------
>> ALTER FOREIGN TABLE pem.asdas INHERIT pemdata.number_of_wal_files;
>>
>> ALTER FOREIGN TABLE pem.asdas NO INHERIT "Test123"."TEST";
>>
>> Done
>
>>
>> *2)* In EDIT mode, Changing just column name of existing column first
>> drops the entire column
>> and adds it again, instead of just renaming column name
>>
>> (*** Consider a case when user have data in the table & just want to
>> rename column col1 to col2
>> or any other attribute like default value/not null etc, why do we need to
>> drop entire column in that case? ***)
>>
>> SQL:
>> ----
>> ALTER FOREIGN TABLE test.test_ft
>> DROP COLUMN c1;
>>
>> ALTER FOREIGN TABLE test.test_ft
>> ADD COLUMN c12 bigint NOT NULL;
>>
>> Expected:
>> ---------
>> ALTER FOREIGN TABLE test.test_ft
>> RENAME c1 TO c12;
>>
>>
>> Done
>
>> *3)* We should not allow user to delete columns inherited from
>> other table in edit mode from UI
>>
>>
>> Done
>
>> *4)* We should not allow user to delete constraint inherited from
>> other table in edit mode from UI
>>
>> Done
>
>>
>> *5)* Issue from my last email still exists, changing type of column
>> creates wrong sql if it has Length/Precision.
>>
>> eg: col12 was numeric(12) which I changed to aclitem
>> See the wrong sql
>>
>> ALTER FOREIGN TABLE pem."test-2"
>> DROP COLUMN col12;
>>
>> ALTER FOREIGN TABLE pem."test-2"
>> ADD COLUMN col12 aclitem(12) NOT NULL COLLATE pg_catalog."default";
>>
>>
>> Expected:
>> ---------
>> ALTER FOREIGN TABLE pem."test-2"
>> ADD COLUMN col12 aclitem;
>>
>> Done
>
>>
>> *6)* EDIT mode, clearing Data Type from column throws Error on console
>>
>> TypeError: this.dataAdapter is null
>>
>> Couldn't reproduce.
>
>>
>> *7)* EDIT mode, If i have blank column row and clicks on SQL tab throws
>> error
>>
>> app_iter = app(environ, start_response)
>> TypeError: 'KeyError' object is not callable
>>
>> Done
>
>>
>> *8)* User can clear Owner which we should not allow to clear OR we have
>> to generate SQL for that opration
>>
>>
>> Done
>
>> *9)* I do not have option to provide Statistics attribute for column
>>
>> Done
>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Wed, May 11, 2016 at 3:17 PM, Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached updated patch for the Foreign Table module.
>>>
>>> Thanks,
>>> Khushboo
>>>
>>> On Tue, May 10, 2016 at 4:33 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Mon, May 9, 2016 at 1:23 PM, Ashesh Vashi <
>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> On Mon, May 9, 2016 at 5:50 PM, Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please find the attached updated patch for the Foreign Tables module.
>>>>>>
>>>>>
>>>> Feedback from my initial testing:
>>>>
>>>> - Owner is missing from the New dialogue.
>>>>
>>>> Done
>>>
>>>> - The Properties list shows "Select from the list" in a combo box for
>>>> Owner and
>>>> Inherits.
>>>>
>>>
>>>
>>>>
>>>>
>>> This is a generalize issue, I will create a new Kanban card for the same.
>>>
>>>> - s/Oid/OID
>>>>
>>>> Done
>>>
>>>> - s/Foreign Server/Foreign server
>>>>
>>>> Done
>>>
>>>> - SQL Help filenames are not defined.
>>>>
>>>> Done
>>>
>>>> - SQL formatting is weird, e.g.
>>>>
>>>> CREATE FOREIGN TABLE public.redis_table
>>>> (id text NOT NULL COLLATE pg_catalog."default",
>>>> value text NULL COLLATE pg_catalog."default")
>>>> SERVER redis_server1
>>>> OPTIONS (database '0');
>>>>
>>>> instead of:
>>>>
>>>> CREATE FOREIGN TABLE public.redis_table (
>>>> id text NOT NULL COLLATE pg_catalog."default",
>>>> value text NULL COLLATE pg_catalog."default"
>>>> )
>>>> SERVER redis_server1
>>>> OPTIONS (database '0');
>>>>
>>>>
>>> Done
>>>
>>>> - In the validation error messages, please replace ! with . and "can
>>>> not" with
>>>> "cannot" for consistency with recently reviewed strings.
>>>>
>>>>
>>> Done
>>>
>>>> - s/mulitple/multiple (in the js).
>>>>
>>>> Done
>>>
>>>> - How do I manage the ACL?
>>>>
>>>> Done. I missed this as I was following pgAdmin-3 as usual.
>>>
>>>> Thanks.
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-14 18:34:20 pgAdmin 4 commit: [Python 3 compatibility] Improved the background proc
Previous Message Murtuza Zabuawala 2016-05-14 17:05:20 Re: PATCH: Initiale backup utility [pgAdmin4]