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-15 07:37:22
Message-ID: CAFOhELceEWd_o0t8GgBTToc3R_6Or2-eEMzQD=XTbQ37_TRPDQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

Please find the updated patch for the foreign table.

Thanks,
Khushboo

On Sat, May 14, 2016 at 10:36 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

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

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

Attachment Content-Type Size
pgAdmin4_foreign_tables_ver6.patch text/x-patch 120.3 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-15 10:34:47 pgAdmin 4 commit: Addd support for taking backup for the server.
Previous Message Ashesh Vashi 2016-05-14 18:34:20 pgAdmin 4 commit: [Python 3 compatibility] Improved the background proc