Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b1d1h-00043f-Ln for pgadmin-hackers@arkaria.postgresql.org; Sat, 14 May 2016 17:06:53 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1b1d1h-0004in-8O for pgadmin-hackers@arkaria.postgresql.org; Sat, 14 May 2016 17:06:53 +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 1b1d1T-0004Uj-Vj for pgadmin-hackers@postgresql.org; Sat, 14 May 2016 17:06:40 +0000 Received: from mail-qg0-x233.google.com ([2607:f8b0:400d:c04::233]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1b1d1L-0008QF-PF for pgadmin-hackers@postgresql.org; Sat, 14 May 2016 17:06:39 +0000 Received: by mail-qg0-x233.google.com with SMTP id f74so72367328qge.2 for ; Sat, 14 May 2016 10:06:31 -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=cJaNgI5S1f9bQ5vm0ARm+mL6sCSpbxMoKC4MXrJwCZc=; b=DuBc5CvKJQIX+fbSQPIXv9f3yUxQY8hS7l82Oq84nXrWKhJ1LPxMQejichcZsD3yeC M86En/82eCIOIBLd8KiQL+c+flFSg3rnjOJ/ILolumP13+6zSDbfQyuCJQ6FxZYC74xc ay1+M717msZiDDbmd1cJOI8wWFR/aNzzfe+v2+g6ZA3sviK119kGCuQ2GYd+HCoSZ6aR X73HMuk2W7x46BS64xqdKd0Xo5YH/sDq8wzG5GShsVGcj1bNHfiN+zA6W9DLYJFnd2Oi ECjrkj7v7FtOJgDuWFrQFdVYCiP9YablfS88XLdvaGieV159jIVwC4eHuJyt70HiPsO3 1mRw== 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=cJaNgI5S1f9bQ5vm0ARm+mL6sCSpbxMoKC4MXrJwCZc=; b=f9UV3jkQ71GVaMiiJ0d5ONlS/W1OcwzyoFSwXawBfcUAfKac3+H1ASPIESgKvP6aIB 5/JRZJAHmYbi5mmFM1swrVq+wmFSaKuZslU0LK8JvUhqlCM1wWvDghSwMvtNrz0Pxaxx mpChNu7CFQjadSsZljgs5orlNqU+VnIGipvWX2da7gq0ikS9Z/zBLpA7lbZxGYwQ5oas dNJKRXuyN9RK1ub/OrzjRcIMwamLri0h9ubz/OBrD6aXDpIa69/Xu2JzNTk8WaBdKeBT 4Tc8r6zG2m9hJxLM4Y8yAe5wwuKb4hCO3DWNyhb7c7i1r3hdEMygL7F5upgbKjWk2q/E OORA== X-Gm-Message-State: AOPr4FWGQ611lfxaPX47L5Li5sls9DyU+txZm28lYmG/9IqtvJDn962/1ZwLBLsSrIPmlz8+hvDD9HwNeaGQ/jw8 X-Received: by 10.140.91.16 with SMTP id y16mr21117123qgd.53.1463245590008; Sat, 14 May 2016 10:06:30 -0700 (PDT) MIME-Version: 1.0 Received: by 10.55.207.202 with HTTP; Sat, 14 May 2016 10:06:10 -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: Sat, 14 May 2016 22:36:10 +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=001a11391c849cb4aa0532d0699e 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 --001a11391c849cb4aa0532d0699e Content-Type: text/plain; charset=UTF-8 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. 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 >>>> >>> >>> >> > --001a11391c849cb4aa0532d0699e Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
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.


Regards,
Murtuza


--
Re= gards,
Murtuza Zabuawala
EnterpriseDB:=C2=A0http://www.enterprisedb.com
The Enterprise= PostgreSQL Company

On Fri, May 13, 2016 at 3:25 PM, Khushboo Va= shi <khushboo.vashi@enterprisedb.com> wrote:
Hi,

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

Thanks,
Khushboo
<= br>
On Fri, May 13, 2016 at 10:50 AM, Murtuza Zab= uawala <murtuza.zabuawala@enterprisedb.com>= wrote:
1) I= f 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=C2=A0
see the wrong drop column sql.

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

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

ALTER FOREIGN TABLE pem.asdas INHERIT pemdata.number_of_wal= _files;

ALTER FOREIGN TABLE pem.asdas NO INHERIT &= quot;Test123"."TEST";

Expected:
---------
ALTER FOREIGN TABLE pem.asdas INHERIT pemdata.nu= mber_of_wal_files;

ALTER FOREIGN TABLE pem.asdas N= O INHERIT "Test123"."TEST";

<= /blockquote>
Done=C2=A0

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=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, why do we need= to drop entire column in that case? ***)

SQL:
----=C2=A0
ALTER FOREIGN TA= BLE test.test_ft
=C2=A0 =C2=A0 DROP COLUMN c1;

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;


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


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

Done=C2=A0

5) Issue from my last email still exists, chang= ing type of column
creates wrong sql if it has Length/Precision.<= /div>

eg: col12 was numeric(12) which I changed to aclit= em=C2=A0
See the wrong sql

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

ALTER FOREIGN TABLE pem."test-2"
=C2=A0 =C2=A0 ADD COLUMN col12 aclitem(12) NOT NULL COLLATE pg_catalog.&qu= ot;default";


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

Done=C2=A0
<= div>

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

TypeError: this.dat= aAdapter is null

Couldn't r= eproduce.=C2=A0
<= /div>

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

=C2=A0 =C2=A0 app= _iter =3D app(environ, start_response)
TypeError: 'KeyError&#= 39; 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

--
Regards,Mur= tuza Zabuawala
EnterpriseDB:=C2=A0http://www.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
Blog: http://pgsnake.blogspot.com
Twitter: @pg= snake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postg= reSQL Company


--001a11391c849cb4aa0532d0699e--