Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b22Pb-0003B6-QR for pgadmin-hackers@arkaria.postgresql.org; Sun, 15 May 2016 20:13:16 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1b22Pb-0003my-DK for pgadmin-hackers@arkaria.postgresql.org; Sun, 15 May 2016 20:13:15 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1b22PN-0003Z1-TY for pgadmin-hackers@postgresql.org; Sun, 15 May 2016 20:13:02 +0000 Received: from mail-io0-x22d.google.com ([2607:f8b0:4001:c06::22d]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1b22PG-00027o-LC for pgadmin-hackers@postgresql.org; Sun, 15 May 2016 20:13:00 +0000 Received: by mail-io0-x22d.google.com with SMTP id i75so183558880ioa.3 for ; Sun, 15 May 2016 13:12:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=5v9Ltzql3cTID0qxABSVlU1rZM6ARuykjFDL7O9y3EU=; b=Krmu8xld2De00NqerL3LiK4w31e/bUygb6542IfXdFR1vqY7Zwnep/VEa8A363DS6h oq5bTUKEXmR1loA3C93hW7+z2hGVfn3BAFSnEvfc4k6Kgapo2Crw0p4owUmcB6x3bM2c oWpUOBlaUau3HAvkVlTRctwwfTvWibM1G7KIIwUYoprbTAAsTbFKtCaimvShxNQTojOd dL5VyycLfjlBQZao2Wd/WY40bFhkTkfhzmj+oW4UQRaI2x4WlwTD1kOkzkoTzWu77g7+ OJKWQhn28FCSmxP8pHgtlXerzQezxYKZcrdMgGJ3QfuceJIlh8j6hE0/Z9Bqsq2QmIV1 I6sg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=5v9Ltzql3cTID0qxABSVlU1rZM6ARuykjFDL7O9y3EU=; b=lCj/3guA7+IpSW8MRwj3UTEqNj3O5qahaXwmBTGtMX2byz+y95zLpPttqQxgpSZHQe gSqhc4QiLcQLIUaK+txB2nZHlKHGFOy2O8zmOgYDaPQV+d++JL6b34OSWza1JMitRRPn 9Br6Dfpt277dI5Z9fiLKCrkcailiO7DJ+4UCgejPPgER6GgnOZgIyZ5GpZ56sCnFV0xY MRQYvYapGKbGQcQEL7Stvux2GmMMjzkG187NvuqhpVVDHQ29sIcHlQsl/uA3VKl/USJP 6mPqdCXRe0G2CcmJGutEFOx4V+jqm4948a8s0LNXQUC7E0vE8DluRHvtTTS/C806HIVw kC8Q== X-Gm-Message-State: AOPr4FWht0J/EAx4n7kNyEXB763WgdfHel/nu3vUyZ9qBnqer+zkTA7qwCltxIZE0zPK5mlXWNd2Pfq9uAOVI3Gm X-Received: by 10.107.161.206 with SMTP id k197mr3815301ioe.149.1463343173680; Sun, 15 May 2016 13:12:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.31.209 with HTTP; Sun, 15 May 2016 13:12:34 -0700 (PDT) In-Reply-To: References: <58B5FCCE-9882-44C0-B990-8BF5D2FA17DC@enterprisedb.com> <928839C0-1CD1-4307-9C62-6A693D6F0533@enterprisedb.com> <2BC9FA40-F05D-48AA-8796-29808FC15498@enterprisedb.com> From: Ashesh Vashi Date: Mon, 16 May 2016 01:42:34 +0530 Message-ID: Subject: Re: [pgAdmin4] [Patch]: Foreign Table Module To: Khushboo Vashi Cc: Murtuza Zabuawala , Dave Page , Neel Patel , pgadmin-hackers Content-Type: multipart/alternative; boundary=001a1140f9880d688b0532e722ae X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a1140f9880d688b0532e722ae Content-Type: text/plain; charset=UTF-8 Thanks - committed! -- Thanks & Regards, Ashesh Vashi EnterpriseDB INDIA: Enterprise PostgreSQL Company *http://www.linkedin.com/in/asheshvashi* On Sun, May 15, 2016 at 1:07 PM, Khushboo Vashi < khushboo.vashi@enterprisedb.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.zabuawala@enterprisedb.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.vashi@enterprisedb.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.zabuawala@enterprisedb.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.vashi@enterprisedb.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 wrote: >>>>> >>>>>> >>>>>> >>>>>> On Mon, May 9, 2016 at 1:23 PM, Ashesh Vashi < >>>>>> ashesh.vashi@enterprisedb.com> wrote: >>>>>> >>>>>>> On Mon, May 9, 2016 at 5:50 PM, Khushboo Vashi < >>>>>>> khushboo.vashi@enterprisedb.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 >>>>>> >>>>> >>>>> >>>> >>> >> > --001a1140f9880d688b0532e722ae Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Thanks - committed!

--

Thanks & Regards,=

Ashesh Vashi
EnterpriseDB INDIA: <= a href=3D"http://www.enterprisedb.com" target=3D"_blank">Enterprise Postgre= SQL Company

<= br>

<= a href=3D"http://www.linkedin.com/in/asheshvashi" target=3D"_blank">http= ://www.linkedin.com/in/asheshvashi


On Sun, May 15, 2016 at 1:07 PM, Khushboo Va= shi <khushboo.vashi@enterprisedb.com> wrote:
Hi,

Pl= ease find the updated patch for the foreign table.

Thanks,
Khushboo

On Sat, May 14, 2016 at 10:36 PM, Murtuza= Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:
Hi,

One last otherwise rest of functionality looks good.<= /div>

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

Fixed=C2=A0

Regards,
Murtuza


--
<= font color=3D"#888888">Murtuza Zabuawala
EnterpriseDB:=C2=A0<= a href=3D"http://www.enterprisedb.com/" rel=3D"noreferrer" style=3D"color:r= gb(17,85,204)" target=3D"_blank">http://www.enterprisedb.com
The Ente= rprise PostgreSQL Company

On Fri, May 13, 2016 at 3:2= 5 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.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.zabuawala@enterprisedb.= com> wrote:
1) If we delete one inherited table and add another one
=
It generated drop columns sql for that inherited table which throws er= ror,
for example "Test123"."TEST" has col1, c= ol2 in it, once we remove table=C2=A0
see the wrong drop column s= ql.

SQL:
----=C2=A0
ALTER FORE= IGN TABLE pem.asdas
=C2=A0 =C2=A0 DROP COLUMN col1;
ALTER FOREIGN TABLE pem.asdas
=C2=A0 =C2=A0 DROP COLU= MN col2;

ALTER FOREIGN TABLE pem.asdas INHERIT pem= data.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=C2=A0

2) In EDIT mo= de, Changing just column name of existing column first drops the entire col= umn
and adds it again, instead of just renaming column name=C2=A0=

(*** Consider a ca= se when user have data in the table & just want to rename column col1 t= o col2=C2=A0
or any other attribute like default value/not null e= tc, why do we need to drop entire column in that case? ***)

SQL:
----=C2=A0
ALTER FOREIGN TABLE test.test_ft
=C2=A0 =C2=A0 DROP COLUMN c1;<= /div>

ALTER FOREIGN TABLE test.test_ft
=C2=A0 = =C2=A0 ADD COLUMN c12 bigint NOT NULL;

Expected:
---------
ALTER FOREIGN TABLE test.test_ft
RENAME c1 TO c12;

<= /div>

Done=C2=A0
3) We should not al= low user to delete columns inherited from=C2=A0
other table in ed= it mode from UI


= Done=C2=A0
=
4) We should not allow user to delete constraint inherited from= =C2=A0
other table in edit mode from UI

Done=C2=A0

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=C2=A0
See the wrong sql

ALTER FOREIGN TABLE pem."test-2"
=C2=A0 =C2=A0 DR= OP COLUMN col12;

ALTER FOREIGN TABLE pem."tes= t-2"
=C2=A0 =C2=A0 ADD COLUMN col12 aclitem(12) NOT NULL COL= LATE pg_catalog."default";


Expected:
---------
ALTER FOREIGN TABLE pem."tes= t-2"
=C2=A0 =C2=A0 ADD COLUMN col12 aclitem;

<= /div>
Done=C2=A0

6) EDIT mode, clear= ing Data Type from column throws Error on console

= TypeError: this.dataAdapter is null

=
Couldn't reproduce.=C2=A0

7) EDIT mode, If i have = blank column row and clicks on SQL tab throws error

=C2=A0 =C2=A0 app_iter =3D app(environ, start_response)
TypeErr= or: 'KeyError' object is not callable

Done=C2=A0

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


Done=C2=A0
9) I do = not have option to provide Statistics attribute for column

Done=C2=A0
=

--
Regar= ds,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The Enterprise P= ostgreSQL Company

On Wed, May 11, 2016 at 3:17 PM, Khushboo Va= shi <khushboo.vashi@enterprisedb.com> wrote:
Hi,

Pl= ease find the attached updated patch for the Foreign Table module.

Thanks,
Khushboo

On Tue, May 10, 2016 at 4:33 PM, Dave Page = <dpage@pgadmin.org> wrote:


On Mon, May 9, 2016 at 1:23 PM, Ashesh Vashi &= lt;ashes= h.vashi@enterprisedb.com> wrote:

On Mon, May 9, 2016 at = 5:50 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com= > wrote:

Hi,

Ple= ase find the attached updated patch for the Foreign Tables module.

Feedback from my initial testing:

- Owner is= missing from the New dialogue.

Done=C2=A0
- T= he Properties list shows "Select from the list" in a combo box fo= r Owner and
=C2=A0 Inherits.
=
=C2=A0
=C2=A0
This is a generalize issue, I will create a new Kanba= n card for the same.
<= div class=3D"gmail_extra">
- s/Oid/OID
=

Done=C2=A0
- s/Foreign Server/Foreign server
<= div>
Done=C2=A0
- SQL Help filenames are not defined.

Done=C2=A0
- SQL formatting is weird, e.g.
<= div>
CREATE FOREIGN TABLE public.redis_table
(id te= xt NOT NULL COLLATE pg_catalog."default",
value text NU= LL COLLATE pg_catalog."default")
=C2=A0 =C2=A0 SERVER r= edis_server1
=C2=A0 =C2=A0 OPTIONS (database '0');
<= div>=C2=A0 =C2=A0=C2=A0
instead of:

CREA= TE FOREIGN TABLE public.redis_table (
=C2=A0 =C2=A0 id text NOT N= ULL COLLATE pg_catalog."default",
=C2=A0 =C2=A0 value t= ext NULL COLLATE pg_catalog."default"
)
=C2= =A0 =C2=A0 SERVER redis_server1
=C2=A0 =C2=A0 OPTIONS (database &= #39;0');
=C2=A0 =C2=A0=C2=A0
Done=C2=A0
- In the validatio= n error messages, please replace ! with . and "can not" with=C2= =A0
=C2=A0 "cannot" for consistency with recently revie= wed strings.
=C2=A0=C2=A0
Done=C2=A0
- s/mulitple/multiple (in= the js).

Done=C2= =A0
- How do I manage the A= CL?=C2=A0

Done. I m= issed this as I was following pgAdmin-3 as usual.=C2=A0
Thanks.


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

<= /span>
<= /span>



--001a1140f9880d688b0532e722ae--