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

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(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-13 09:55:20
Message-ID: CAFOhELcKCCAMQYCrSj0Wz17DozYC+RrFGsVfTy8P6gjjVQ3Evw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>>>
>>
>>
>

Attachment Content-Type Size
pgAdmin4_foreign_table_ver5.patch text/x-patch 120.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Harshal Dhumal 2016-05-13 10:17:07 psycopg2 issue when returning password from sqlite db to python [pgadmin4]
Previous Message Akshay Joshi 2016-05-13 09:37:59 pgAdmin 4 commit: Fixed following issues: