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

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, Dave Page <dpage(at)pgadmin(dot)org>, 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 20:12:34
Message-ID: CAG7mmoz8S_qyUjzsCgjEpVsb4ufNbW=JukR8X+DoK5OciwzL2A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks - committed!

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com>

*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi>

On Sun, May 15, 2016 at 1:07 PM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

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

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-15 20:28:16 pgAdmin 4 commit: Adding initial version for showing statistics for the
Previous Message Ashesh Vashi 2016-05-15 20:12:20 pgAdmin 4 commit: Add support for foreign server