Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b15Wz-0007qB-9T for pgadmin-hackers@arkaria.postgresql.org; Fri, 13 May 2016 05:20:57 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1b15Wy-00069Z-4N for pgadmin-hackers@arkaria.postgresql.org; Fri, 13 May 2016 05:20:56 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1b15Wk-0005vG-Qw for pgadmin-hackers@postgresql.org; Fri, 13 May 2016 05:20:42 +0000 Received: from mail-qk0-x22d.google.com ([2607:f8b0:400d:c09::22d]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1b15Wg-0002o7-KP for pgadmin-hackers@postgresql.org; Fri, 13 May 2016 05:20:42 +0000 Received: by mail-qk0-x22d.google.com with SMTP id n63so55217417qkf.0 for ; Thu, 12 May 2016 22:20:38 -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=UMSNYMe1ynI5w00cVLmPyMX3l1zf/KaAHuQSCwI6nXw=; b=K51SXYfo0WCKit8eEDAeFFEfGHaKk3algTLR6HiLTrqOkho4M+m4/iUNVNZfZ9QeWx Ca3xTs2LNpBp9sg6+8c7A9esqHAJlW3o562Cs8Pa0da82jX/q77qSd9Pl7vDCQ3sUNYB 5VYEzKEpSZK+yRCf74rxsIgvvQ0C41AcCRz+PCoEkCVFkmTG9CQeXbxv9etBOvjiT2Xg xkvV6HLfPuy2p+4v2nikIDB8EC4nzLhTYAjq0kbgR3s0Osv83L1vFC7LusfUuLEWnIrv rjd05PwOit+DA6VuvYJ0vcYkZHy8t61EunpmQ9lxEb1p2pEqYGVioUpf8wZgjtXJCJ7t oFzw== 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=UMSNYMe1ynI5w00cVLmPyMX3l1zf/KaAHuQSCwI6nXw=; b=Pm1ENqiCCHqOyU0EhpbbAXeCWzaTUFL+G2TcxCdE8DmnSO4JuQMlgV/dShqMkw0Se8 OUlaCTWytpt378l9CUMsKh+YYva8RFOom+L+bVMDEM+o7i1pgzpLjhUHNDMGISiyC1yW eXAOtRoZlcKQFXwjHMvVbmgOt8Ba48j5I/dS56UtjX9uOwzUZTqJE16R+Ws8pkjZdK+V AZ3hPjQr0ZvziTtlEBn3NRe0Y6TuG7e+/47nw2td6AOLCMwM2CvkMSrZbxN0mNrNb0zh LIrAuGb8tur8e+sdKRS5m4gQcTpkTaL2EolNAH0oWgIHpHCZBD83dW2Wcyhs8IkD9RYE M5rg== X-Gm-Message-State: AOPr4FU0qDDf/+lDFMvJ0uj9qP7wkZLQlkzgVpDBHgfFHX1xoeSmuGCRw5MuB4/B7Wt/sGslQdHxdZe8x+YcDs0/ X-Received: by 10.55.88.5 with SMTP id m5mr13866457qkb.134.1463116836849; Thu, 12 May 2016 22:20:36 -0700 (PDT) MIME-Version: 1.0 Received: by 10.55.215.157 with HTTP; Thu, 12 May 2016 22:20:17 -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: Murtuza Zabuawala Date: Fri, 13 May 2016 10:50:17 +0530 Message-ID: Subject: Re: [pgAdmin4] [Patch]: Foreign Table Module To: Khushboo Vashi Cc: Dave Page , Ashesh Vashi , Neel Patel , pgadmin-hackers Content-Type: multipart/alternative; boundary=94eb2c0bacb0535f4d0532b26fd0 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 --94eb2c0bacb0535f4d0532b26fd0 Content-Type: text/plain; charset=UTF-8 *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.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 >> > > --94eb2c0bacb0535f4d0532b26fd0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
1) If we delete one inherited table and add an= other one
It generated drop columns sql for that inherited table = which throws error,
for example "Test123"."TEST&qu= ot; has col1, col2 in it, once we remove table=C2=A0
see the wron= g drop column sql.

SQL:
----=C2=A0
=
ALTER FOREIGN TABLE pem.asdas
=C2=A0 =C2=A0 DROP COLUMN col1= ;

ALTER FOREIGN TABLE pem.asdas
=C2=A0 = =C2=A0 DROP COLUMN col2;

ALTER FOREIGN TABLE pem.a= sdas INHERIT pemdata.number_of_wal_files;

ALTER FO= REIGN 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&= quot;;


2) In EDIT mode, Chan= ging just column name of existing column first drops the entire column
and adds it again, instead of just renaming column name=C2=A0

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

SQL:
----=C2=A0
ALTE= R 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>

3) We should not allow user to delete colum= ns inherited from=C2=A0
other table in edit mode from UI


4) We should not allow user to delet= e constraint inherited from=C2=A0
other table in edit mode from U= I


5) Issue from my last emai= l still exists, changing type of column
creates wrong sql if it h= as Length/Precision.

eg: col12 was numeric(12) whi= ch I changed to aclitem=C2=A0
See the wrong sql

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

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


<= div>Expected:
---------
ALTER FOREIGN TABLE pem."t= est-2"
=C2=A0 =C2=A0 ADD COLUMN col12 aclitem;

6) EDIT mode, clearing Data Type from col= umn throws Error on console

TypeError: this.dataAd= apter is null


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)
TypeError: 'KeyError' object is not callable


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


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

<= br clear=3D"all">
--
Regards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://ww= w.enterprisedb.com
The Enterprise PostgreSQL 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<= span>
Blog: ht= tp://pgsnake.blogspot.com
Twitter: @pgsnake

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


--94eb2c0bacb0535f4d0532b26fd0--