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-13 05:20:17
Message-ID: CAKKotZQAxEppv8eMJPCcE9qi0=Nu49su=QjdMzdS+5Hk2sYGxA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

*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";

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

*3)* We should not allow user to delete columns inherited from
other table in edit mode from UI

*4)* We should not allow user to delete constraint inherited from
other table in edit mode from UI

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

*6)* EDIT mode, clearing Data Type from column throws Error on console

TypeError: this.dataAdapter is null

*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

*8)* User can clear Owner which we should not allow to clear OR we have to
generate SQL for that opration

*9)* I do not have option to provide Statistics attribute for column

--
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 Akshay Joshi 2016-05-13 05:53:17 pgAdmin 4 commit: Fixed cStringIO and StringIO module compatibility in
Previous Message Neel Patel 2016-05-13 05:10:53 [pgAdmin4][patch] : cStringIO and StringIO module compatibility in python