Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1azkEV-0001Vv-7f for pgadmin-hackers@arkaria.postgresql.org; Mon, 09 May 2016 12:24:19 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1azkEU-0006m6-QS for pgadmin-hackers@arkaria.postgresql.org; Mon, 09 May 2016 12:24:18 +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 1azkEH-0006Y0-2O for pgadmin-hackers@postgresql.org; Mon, 09 May 2016 12:24:05 +0000 Received: from mail-ig0-x230.google.com ([2607:f8b0:4001:c05::230]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1azkEA-0008DN-VW for pgadmin-hackers@postgresql.org; Mon, 09 May 2016 12:24:04 +0000 Received: by mail-ig0-x230.google.com with SMTP id m9so90107138ige.1 for ; Mon, 09 May 2016 05:23:58 -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=7V11tgvMNYOX0W1KO8VwZ3th3Fv9BI5e/Vv19Ib+TMc=; b=jwpOytAJPHPNrd/b5MCWb6Pm6D4ljuO7D6S08ZZqNIXCBiR7c2UO78IWDqVDKiKFCA IFB4fVvAmLlV52v111msPszTIw1hkgDh1LmIxDXgtu04z+8WZoLkMYsHCqKwU4bWglIN 2eF0YCBCSQjyxdYs2LQp6pH7H6YSFFhCVmpMfTd3ZRfV5BCdWuJ/HQyqk+CKkZ8vc2nj G4cH9onLTCg7c5QXvrF+I4kByRQcRrSWolJiQ2x3niP0cF6jdmW6zM99ER80/gY7/FAY oVZZwe8u6q2SRRS7Mkyq5949WkxPFUgBnCURbXf3eCRuBQsjRTxUg7lTRNJ+VvFu97fL lULw== 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=7V11tgvMNYOX0W1KO8VwZ3th3Fv9BI5e/Vv19Ib+TMc=; b=hZXJbbIxinwEBJDzgsOanKK9FBT+RnXUgfGSab5fQ765WFf6uGy91HJUYTksmDIAqM IBdHSOQNmfhGBzod6PMO40czh5AIPU5ATLyQysaFlXy0TB8SejBjrZP+LvWv4xJqbYtb IhvZz8ZaCjOB4aFvNyBf116nT9IcGo6nA334m0Acle2Zc7q2wllN/LdfcpjxO3504m3F tYz/rNz9RwXOCoHWq0u673+FNCerRdr5gWSyPwc9K7b0urnCPmk40k8QoinKtmDvhjbb ElL96HejiDFkuZKEJCHrvLopmGhyIaLv0ydUoRlxmdAM+DBlpdlT9+amtpbhjluNX2wJ ZWTw== X-Gm-Message-State: AOPr4FXuZTP8kj73B1U/LkrrLyei9Dhxk4oqqWs7dJ31VOiM6SuqcTpdCImShTGInGZ8QlU9bz62syYw4Z8W7tw3 X-Received: by 10.50.69.4 with SMTP id a4mr10601262igu.70.1462796634790; Mon, 09 May 2016 05:23:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.31.209 with HTTP; Mon, 9 May 2016 05:23:35 -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, 9 May 2016 17:53:35 +0530 Message-ID: Subject: Re: [pgAdmin4] [Patch]: Foreign Table Module To: Khushboo Vashi Cc: Murtuza Zabuawala , Neel Patel , pgadmin-hackers Content-Type: multipart/alternative; boundary=047d7bfe9abacbdd55053267e160 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 --047d7bfe9abacbdd55053267e160 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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. > > Thanks, > Khushboo > > On Mon, May 9, 2016 at 1:46 PM, Murtuza Zabuawala < > murtuza.zabuawala@enterprisedb.com> wrote: > >> >> Hi Khushboo, >> >> Please find comments as below, >> (Tested with Python3) >> >> *1)* Default string value is not quoted properly which causing SQL to >> break, here default string should be 'my test string' for myCol2. >> >> CREATE FOREIGN TABLE "Test123"."test1_Server" >> ("myCol" bigint NOT NULL DEFAULT 213 COLLATE pg_catalog."C", >> "myCol2" character varying(50) NOT NULL DEFAULT my test string COLLATE >> pg_catalog."C.UTF-8") >> INHERITS ("Test123".abc, pem.chart) >> SERVER test_fw_server; >> >> I think this behavior is perfect as we can't add single quotes for all > the data-types. User should have to add himself if required. > This is correct behaviour as of now. Default value can be an expression too. Hence - we need to rely on the user to provide correct value here. We may improve this in the future. -- Thanks & Regards, Ashesh Vashi EnterpriseDB INDIA: Enterprise PostgreSQL Company *http://www.linkedin.com/in/asheshvashi* > > >> *2)* When Length & Precision, does not clear when user selects another >> data type >> - First select Numeric provide Length & Precision and then choose abstim= e >> data type which have neither Length & Precision >> - It will not clear/ not allow user to clear old values & generates wron= g >> SQL >> >> ALTER FOREIGN TABLE "Test123"."test1_TT1" >> ADD COLUMN col1 abstime(50 , 2) NOT NULL COLLATE pg_catalog."C"; >> > Done > >> *3)* I am allowed to select self table as inherited table >> >> ALTER FOREIGN TABLE pem."test1_TT1" INHERIT pem."test1_TT1"; >> >> Done > >> *4)* Wrong SQL generated for array like data types >> >> ALTER FOREIGN TABLE pem."test1_TT1" >> ADD COLUMN name character[](50 ) NOT NULL COLLATE pg_catalog."POSIX"= ; >> >> Correct SQL: >> >> ALTER FOREIGN TABLE pem."test1_TT1" >> ADD COLUMN name character(50)[] NOT NULL COLLATE >> pg_catalog."POSIX"; >> >> Done > >> *5)* I am allowed to enter duplicate options but as per postgres >> documentation >> we should not allow duplicate options >> >> ALTER FOREIGN TABLE pem."test1_TT1" >> OPTIONS (ADD op1 'val1'); >> >> ALTER FOREIGN TABLE pem."test1_TT1" >> OPTIONS (ADD op1 'val1'); >> >> ALTER FOREIGN TABLE pem."test1_TT1" >> OPTIONS (ADD op1 'val2'); >> >> Done > >> *6)* Created table with Special name (eg: table name =3D> "@Test#" ) & i= t >> breaks when we clicks on it. >> >> File >> "/home/murtuza/projects/pgadmin4/web/pgadmin/browser/server_groups/serve= rs/databases/schemas/foreign_tables/__init__.py", >> line 414, in properties >> data =3D self._fetch_properties(gid, sid, did, scid, foid) >> File >> "/home/murtuza/projects/pgadmin4/web/pgadmin/browser/server_groups/serve= rs/databases/schemas/foreign_tables/__init__.py", >> line 982, in _fetch_properties >> c['typlen'] =3D int(typlen) >> TypeError: int() argument must be a string, a bytes-like object or a >> number, not 'list' >> > >> > Couldn't reproduce but I think while fixing above issues it is resolved. > >> *7) *While dropping any foreign table gives this error but table gets >> deleted from browser tree. >> >> s/databases/schemas/foreign_tables/__init__.py", line 414, in properties >> data =3D self._fetch_properties(gid, sid, did, scid, foid) >> File >> "/home/murtuza/projects/pgadmin4/web/pgadmin/browser/server_groups/serve= rs/databases/schemas/foreign_tables/__init__.py", >> line 982, in _fetch_properties >> c['typlen'] =3D int(typlen) >> TypeError: int() argument must be a string, a bytes-like object or a >> number, not 'list' >> > Couldn't reproduce but I think while fixing above issues it is resolved. > >> >> *Can be added into TODO:* >> *=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D* >> >> *1)* We should not allowed user to Inherits from catalog tables like >> pg_catalog.pg_type; >> >> *2)* Minor SQL alignment when only server & table name was given >> >> CREATE FOREIGN TABLE "Test123"."test1_Server" >> () >> SERVER test_fw_server; >> >> Expected: >> --------- >> CREATE FOREIGN TABLE "Test123"."test1_Server" () >> SERVER test_fw_server; >> >> *3)* Collation client side validation missing for columns, not every >> data type support Collations, >> For other data types it should be disable just like Length & Precisio= n. >> >> >> Regards, >> Murtuza >> >> >> >> On 09-May-2016, at 10:43 am, Murtuza Zabuawala < >> murtuza.zabuawala@enterprisedb.com> wrote: >> >> Hi Khushboo, >> >> No I did not. let me apply it and try again. >> >> Thanks, >> Murtuza >> >> >> On 06-May-2016, at 10:15 pm, Khushboo Vashi < >> khushboo.vashi@enterprisedb.com> wrote: >> >> Hi Murtuza, >> >> Have you applied Dependent Cell patch on this? As the Foreign table is >> dependent on that. >> >> Thanks, >> Khushboo >> On 6 May 2016 20:39, "Murtuza Zabuawala" < >> murtuza.zabuawala@enterprisedb.com> wrote: >> >>> Hi Khushboo, >>> >>> Please find comments as below, >>> >>> I pulled latest version of code and then I applied v2 patch. >>> >>> >>> 1) Once applying patch, When I re-started pgAdmin4 server again, I got >>> below error (Screenshot is also attached), >>> * ( FYI, I was not able to re-produce it again second time. )* >>> >>> >>> >>> >>> >>> * File "/Users/murtuza/Desktop/pgadmin4/web/pgadmin/utils/__init__.py"= , >>> line 37, in create_module_preference self.preference =3D >>> Preferences(self.name , None) File >>> "/Users/murtuza/Desktop/pgadmin4/web/pgadmin/utils/preferences.py", lin= e >>> 261, in __init__ db.session.commit()=E2=80=A6..* >>> >>> *=E2=80=A6..* >>> >>> * cursor.execute(statement, >>> parameters)sqlalchemy.exc.OperationalError: (OperationalError) database= is >>> locked 'INSERT INTO module_preference (name) VALUES (?)' >>> ('NODE-foreign-table=E2=80=99,)* >>> >>> >>> 2) I am not able to create/open Foreign table node as I am getting >>> errors from JS side (when I expand the schema node & do not get Create >>> context menu as well) >>> Please find screenshots for both scenario. >>> >>> >>> >> 07-59-45.png> >>> >>> >>> On 06-May-2016, at 3:55 pm, Khushboo Vashi < >>> khushboo.vashi@enterprisedb.com> wrote: >>> >>> Hi, >>> >>> Please find the attached patch for the Foreign Table Node with fixed >>> issues. >>> >>> Thanks, >>> Khushboo >>> >>> On Fri, Apr 29, 2016 at 12:20 PM, Neel Patel < >>> neel.patel@enterprisedb.com> wrote: >>> >>>> Hi Khushboo, >>>> >>>> Below are the observations. >>>> >>>> - When we create the new Foreign Table with column name and types >>>> then it shows NULL along with column name and type in properties ta= b. >>>> >>>> e.g. column_1 xml[] NULL >>>> >>> I think if its NOT NULL, then it should be NULL and it is default, so >>> this should be okay. >>> >>>> >>>> - Once we inherits the table from another table then column and >>>> another parameters of inherited table should not allowed to change. >>>> >>>> Done >>> >>>> >>>> - When we create any foreign table then same foreign table is also >>>> listed under "Tables" node. >>>> >>>> This bug is related to Table node and Harshal is working on this issue= . >>> >>>> >>>> - SQL is not generated properly. Please find below SQL which gives >>>> error during execution. >>>> >>>> CREATE FOREIGN TABLE public.test_2 >>>> (id1 integer NOT NULL DEFAULT12 COLLATEpg_catalog."POSIX") >>>> SERVER fsrv; >>>> >>> Done >>> >>>> >>>> - When we create the new foreign table with security label then no >>>> SQL is generated for security label. >>>> >>>> Done >>> >>>> >>>> - In Edit mode, when we provide security label with both value >>>> "provider" and "security label" then security label is displayed NU= LL >>>> >>>> e.g. SECURITY LABEL FOR frgn_table ON FOREIGN TABLE >>>> public.fsrv_table IS NULL; >>>> >>> Done >>> >>>> >>>> - During creation of the column, when we remove the collation then >>>> it gives below error. >>>> >>>> TypeError: item is undefined >>>> >>> Done >>> >>>> >>>> - Delete/Drop cascade functionality is not working, we are getting >>>> below error. >>>> >>>> TypeError: self.canDrop.apply is not a function >>>> >>> Done >>> >>>> >>>> - When we edit the foreign table and try to remove the existing >>>> "Data Type" of column then it gives below error. >>>> >>>> TypeError: this.dataAdapter is null >>>> >>> Done >>> >>>> >>>> - Create the new foreign table and click on ADD button in Column >>>> tab and do not provide any column name and data type. Need to do pr= oper >>>> validation in Column tab for all parameters. Currently if user do n= ot >>>> provide any value then wrong SQL is getting generated. >>>> >>>> CREATE FOREIGN TABLE public.test_4 >>>> (None None NULL) >>>> SERVER test_fsrv; >>>> >>> Done >>> >>>> >>>> - When we do not provide the Check parameters in constraint then it >>>> gives SQL syntax error. >>>> >>>> CREATE FOREIGN TABLE public.test_5 >>>> () >>>> SERVER test_fsrv; >>>> >>>> ALTER FOREIGN TABLE public.test_5 >>>> ADD CONSTRAINT test CHECK () NOT VALID; >>>> >>> Done >>> >>>> >>>> - If we edit foreign table and change the schema then it gives >>>> below error. >>>> >>>> IndexError: list index out of range >>>> >>> Done >>> >>>> >>>> - We should have proper indentation in the SQL tab once we give the >>>> parameters. Currently it looks like below for "Options" value. >>>> >>>> CREATE FOREIGN TABLE "1_test"."5_test" >>>> () >>>> SERVER asas >>>> OPTIONS (test1 'test2' >>>> , test3 'test4'); >>>> >>> Done >>> >>>> >>>> - If user provide foreign table name and do not provide foreign >>>> server and click on SQL tab then we are getting error on browser si= de as >>>> below. We should have proper error handling in this case. >>>> >>>> Couldn't find the required parameter (ftsrvname). >>>> >>> Done >>> >>>> >>>> - Create the foreign table, add the constraint and do not provide >>>> any constraint information then SQL generated is wrong. >>>> >>>> CREATE FOREIGN TABLE "1_test"."9_test" >>>> () >>>> SERVER test_fsrv; >>>> >>>> ALTER FOREIGN TABLE "1_test"."9_test" >>>> ADD CONSTRAINT None CHECK (); >>>> >>> Done >>> >>>> >>>> - When we click on the foreign table collection node then "Comment" >>>> column is blank even though we have comment in the foreign table. >>>> >>>> Done >>> >>>> >>>> - Create the foreign table on PG 9.1 and after pressing Save button >>>> we are getting below error. >>>> >>>> "the JSON object must be str, not 'list'" >>>> >>> Done >>> >>>> >>>> - When we delete the options parameters then it gives SQL error >>>> because DROP statement does not include the value. >>>> >>>> ALTER FOREIGN TABLE public.test_12 >>>> OPTIONS ( DROP ser2 'val2'); >>>> >>> Done >>> >>>> >>>> - There are some new functionality added in PG 9.5. Do we really >>>> need to implement ? Need to discuss with Dave/Ashesh. Below are the= new >>>> functionality. >>>> >>>> - In create foreign table,we have added column >>>> constraint but "table constraint" is added from 9.5.Do >>>> we really require to add table constraint ? >>>> - In alter foreign table, below are the new >>>> functionality added. >>>> 1. ALTER [ COLUMN ] column_name SET STORAGE { >>>> PLAIN | EXTERNAL | EXTENDED | MAIN } >>>> 2. DISABLE TRIGGER [ trigger_name | ALL | USER ] >>>> 3. ENABLE TRIGGER [ trigger_name | ALL | USER ] >>>> 4. ENABLE REPLICA TRIGGER trigger_name >>>> 5. ENABLE ALWAYS TRIGGER trigger_name >>>> 6. SET WITH OIDS >>>> 7. SET WITHOUT OIDS >>>> >>>> As per the discussion, we will add these functionality into the next >>> phase. >>> >>>> Do let us know in case of any queries. >>>> >>>> Thanks, >>>> Neel Patel >>>> >>>> On Tue, Apr 5, 2016 at 2:27 PM, Khushboo Vashi < >>>> khushboo.vashi@enterprisedb.com> wrote: >>>> >>>>> Hi, >>>>> >>>>> Please find updated patch for the Foreign Table Module. >>>>> >>>>> This patch is dependent on >>>>> 1. Backgrid Depscell Patch, (submitted by me) >>>>> 2. NodeAjaxOptionsCell Transform change patch, on which Ashesh and >>>>> Murtuza are working >>>>> >>>>> Thanks, >>>>> Khushboo >>>>> >>>>> >>>>> >>>>> >>>>> On Wed, Feb 24, 2016 at 2:57 PM, Khushboo Vashi < >>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> I have updated the Foreign Table module as below: >>>>>> >>>>>> - Used 'NodeByListControl' to get schemas, in the foreign_table.js >>>>>> file as suggested by Ashesh to avoid code redundancy. >>>>>> >>>>>> - Applied *'Security Label Macro'* Patch (Implemented by Harshal). >>>>>> To test the Foreign Table patch, 'Security Label Macro' patch must >>>>>> be applied first as that is not committed yet. >>>>>> >>>>>> Please find attached Foreign Table Patch. >>>>>> >>>>>> Thanks, >>>>>> Khushboo >>>>>> >>>>>> On Tue, Feb 23, 2016 at 6:53 PM, Khushboo Vashi < >>>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Please find attached patch for the Foreign Table Module. >>>>>>> >>>>>>> The patch will be modified after Types module implementation as we >>>>>>> need to populate Base Type and some Type related validations from = the >>>>>>> Types module. >>>>>>> >>>>>>> Please review it and let me know the feedback. >>>>>>> >>>>>>> Thanks, >>>>>>> Khushboo >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org= ) >>>>> To make changes to your subscription: >>>>> http://www.postgresql.org/mailpref/pgadmin-hackers >>>>> >>>>> >>>> >>> >>> -- >>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) >>> To make changes to your subscription: >>> http://www.postgresql.org/mailpref/pgadmin-hackers >>> >>> >>> >> >> > > > -- > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > > --047d7bfe9abacbdd55053267e160 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On Mon, May 9, 2016 at 5:50 PM, Khushboo Vashi <<= a href=3D"mailto:khushboo.vashi@enterprisedb.com" target=3D"_blank">khushbo= o.vashi@enterprisedb.com> wrote:

Hi,
Please find the attached updated patch for the Foreign Tables m= odule.

Thanks,
Khushboo

On Mon, Ma= y 9, 2016 at 1:46 PM, Murtuza Zabuawala <murtuza.zabuawal= a@enterprisedb.com> wrote:

Hi Khushboo,

Please find comments as below,
(Tested with Python3)
1) Default string value is not quoted properly which= causing SQL to break, here default string should be 'my test string= 9; for myCol2.

CREATE FOREIGN TABLE "Test123&= quot;."test1_Server"
("myCol" bigint NOT NULL= DEFAULT 213 COLLATE pg_catalog."C",
"myCol2"= character varying(50) NOT NULL DEFAULT my test string COLLATE pg_catalog.&= quot;C.UTF-8")
=C2=A0 =C2=A0 INHERITS ("Test123".a= bc, pem.chart)
=C2=A0 =C2=A0 SERVER test_fw_server;
I think this behavior is perfec= t as we can't add single quotes for all the data-types. User should hav= e to add himself if required.
This= is correct behaviour as of now.

Default value can= be an expression too.
Hence - we need to rely on the user to pro= vide correct value here.

We may improve this in th= e future.

--=

Thanks & R= egards,

Ashesh Vashi
EnterpriseDB INDIA:=C2=A0Enterprise P= ostgreSQL Company


http://www.linkedin.com/in/asheshvashi= =C2=A0
=C2=A0
2) When Length &a= mp; Precision, does not clear when user selects another data type
- First select Numeric provide Length & Precision and then choose abst= ime=C2=A0
=C2=A0 data type which have neither Length & Precis= ion=C2=A0
- It will not clear/ not allow user to clear old values= & generates wrong SQL

ALTER FOREIGN TABLE &qu= ot;Test123"."test1_TT1"
=C2=A0 =C2=A0 ADD COLUMN c= ol1 abstime(50 , 2) NOT NULL COLLATE pg_catalog."C";
<= /div>
Done=C2=A0
3) I am allowed t= o select self table as inherited table

ALTER FOREI= GN TABLE pem."test1_TT1" INHERIT pem."test1_TT1";
=

Done=C2=A0
4) Wrong SQL generated for array like data types
=
ALTER FOREIGN TABLE pem."test1_TT1"
=C2= =A0 =C2=A0 ADD COLUMN name character[](50 ) NOT NULL COLLATE pg_catalog.&qu= ot;POSIX";

Correct SQL:
=C2=A0 =C2= =A0
ALTER FOREIGN TABLE pem."test1_TT1"
=C2= =A0 =C2=A0 =C2=A0 =C2=A0ADD COLUMN name character(50)[] NOT NULL COLLATE pg= _catalog."POSIX";

Done=C2=A0
5) I am allowed to enter d= uplicate options but as per postgres documentation=C2=A0
=C2=A0 = =C2=A0we should not allow duplicate options=C2=A0

= ALTER FOREIGN TABLE pem."test1_TT1"
=C2=A0 =C2=A0 OPTIO= NS (ADD op1 'val1');

ALTER FOREIGN TABLE p= em."test1_TT1"
=C2=A0 =C2=A0 OPTIONS (ADD op1 'val1= ');

ALTER FOREIGN TABLE pem."test1_TT1&qu= ot;
=C2=A0 =C2=A0 OPTIONS (ADD op1 'val2');
Done=C2=A0
6) Created table with Special name (eg: table name =3D> "@T= est#" ) & it breaks when we clicks on it.

=C2=A0 File "/home/murtuza/projects/pgadmin4/web/pgadmin/browser/serv= er_groups/servers/databases/schemas/foreign_tables/__init__.py", line = 414, in properties
=C2=A0 =C2=A0 data =3D self._fetch_properties(= gid, sid, did, scid, foid)
=C2=A0 File "/home/murtuza/projec= ts/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/for= eign_tables/__init__.py", line 982, in _fetch_properties
=C2= =A0 =C2=A0 c['typlen'] =3D int(typlen)
TypeError: int() a= rgument must be a string, a bytes-like object or a number, not 'list= 9;=C2=A0
=C2=A0
Couldn't reproduce but I think while fixing above issues it is resolve= d.
7) While=C2=A0dropping any foreign table giv= es this error but table gets deleted from browser=C2=A0tree.

=
s/databases/schemas/foreign_tables/__init__.py", line = 414, in properties
=C2=A0 =C2=A0 data =3D self._fetch_properties(= gid, sid, did, scid, foid)
=C2=A0 File "/home/murtuza/projec= ts/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/for= eign_tables/__init__.py", line 982, in _fetch_properties
=C2= =A0 =C2=A0 c['typlen'] =3D int(typlen)
TypeError: int() a= rgument must be a string, a bytes-like object or a number, not 'list= 9;
=C2=A0Couldn't repro= duce but I think while fixing above issues it is resolved.
=
Can be added into TODO:
=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

<= /div>
1) We should not allowed user to Inherits from catalog tab= les like pg_catalog.pg_type;

2) Minor SQL a= lignment when only server & table name was given=C2=A0

CREATE FOREIGN TABLE "Test123"."test1_Server"<= /div>
()
=C2=A0 =C2=A0 SERVER test_fw_server;

<= /div>
Expected:
---------
CREATE FOREIGN TABLE &quo= t;Test123"."test1_Server" ()
=C2=A0 =C2=A0 SERVER = test_fw_server;

3) Collation client side va= lidation missing for columns, not every data type support Collations,
=
=C2=A0 =C2=A0For other data types it should be disable just like Lengt= h & Precision.


Regards,
Murtuza



On 09-May-2016, at 10:43 am, Murtuza Zabuawala &= lt;= murtuza.zabuawala@enterprisedb.com> wrote:

Hi Khushboo,

No I did not. let= me apply it and try again.

Thanks,
Murt= uza


O= n 06-May-2016, at 10:15 pm, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Murtuza,

Hav= e you applied Dependent Cell patch on this? As the Foreign table is depende= nt on that.

Thanks,
Khushboo

On 6 May 2016 20:39, "Murtuza Zabuawala&quo= t; <murtuza.zabuawala@enterprisedb.com> wrote:
Hi Khushb= oo,

Please find comments as below,

<= /div>
I pulled latest version of code and then I applied v2 patch.


1) Once applying patch, When I re-start= ed pgAdmin4 server again, I got below error (Screenshot is also attached),<= /div>
=C2=A0 =C2=A0( FYI, I was not able to re-prod= uce it again second time. )

=C2=A0 File "/Users/murtuza/Desktop/pgadmin4/web/pg= admin/utils/__init__.py", line 37, in create_module_preference
=C2= =A0 =C2=A0 self.preference =3D Preferences(self.name, None)
=C2=A0 File "/Users/murtuza/De= sktop/pgadmin4/web/pgadmin/utils/preferences.py", line 261, in __init_= _
=C2=A0 =C2=A0 db.session.commit()
=E2=80=A6..
<= div>=E2=80=A6..
=C2=A0 =C2=A0 cursor.execute(statement, parameters= )
sqlalchemy.exc.OperationalError: (OperationalError) database is locked= 'INSERT INTO module_preference (name) VALUES (?)' ('NODE-forei= gn-table=E2=80=99,)


<= /i>
2) I am not able to create/open Foreign table node as I = am getting errors from JS side =C2=A0(when I expand the schema node & d= o not get Create context menu as well)
=C2=A0 =C2=A0 Please= find screenshots for both scenario.


<Screenshot from 2016-05-06 08-00-19.png><Scr= eenshot from 2016-05-06 07-59-45.png><Screen Shot 2016-0= 5-06 at 8.18.12 pm.png>


On 06-May-2016, at 3:55 pm, Khushboo Vashi <khushboo.vash= i@enterprisedb.com> wrote:

Hi,
Please find the attached patch for the Foreign Table Node with= fixed issues.

Thanks,
Khushboo

On Fri, Apr 29, 2016 a= t 12:20 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi Khushboo,

<= div>Below are the observations.
  • When we create the new Fo= reign Table with column name and types then it shows NULL along with column= name and type in properties tab.
=C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0e.g.=C2=A0column_1 xml[] NULL=C2=A0
<= /blockquote>
I think if its NOT NULL, then it should be NULL and it is = default, so this should be okay.
  • Once we inherits the table from another table then column an= d another parameters of inherited table should not allowed to change.
    • <= /ul>
Done
  • When we create any foreign table then same foreign ta= ble is also listed under "Tables" node.
This bug is related to Table node and Harshal is working on t= his issue.
  • SQL is = not generated properly. Please find below SQL which gives error during exec= ution.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CREATE FOREIG= N TABLE public.test_2
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(i= d1 integer NOT NULL DEFAULT12 COLLATEpg_catalog."POSIX")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0SERVER fsrv;
=
Done
  • When we create the new foreign table with security label t= hen no SQL is generated for security label.
Done
  • In Ed= it mode, when we provide security label with both value "provider"= ; and "security label" then security label is displayed NULL
    • =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 e.g.=C2=A0 SECURITY LAB= EL FOR frgn_table ON FOREIGN TABLE public.fsrv_table IS NULL;
Done
=
  • During creation of the column, when we r= emove the collation then it gives below error.
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0TypeError: item is undefined
=
Done
  • Delete/Drop cascade functionality is not working, we are g= etting below error.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0TypeError: self.canDrop.apply is not a function
Done
=
  • When we edit the foreign table and try t= o remove the existing "Data Type" of column then it gives below e= rror.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TypeEr= ror: this.dataAdapter is null
Done <= br>
    <= li>Create the new foreign table and click on ADD button in Column tab and d= o not provide any column name and data type. Need to do proper validation i= n Column tab for all parameters. Currently if user do not provide any value= then wrong SQL is getting generated.
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0CREATE FOREIGN TABLE public.test_4
=C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(None None NULL)
=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SERVER test_fsrv;
Done
=
  • When we do not provide the Check parameters in constraint then it g= ives SQL syntax error.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0CREATE FOREIGN TABLE public.test_5
=C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0()
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0SERVER test_fsrv;

=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 ALTER FOREIGN TABLE public.test_5
=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ADD CONSTRAINT test CHECK () NOT VALID;=
Done
=
  • If we edit foreign table and change the schem= a then it gives below error.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 IndexError: list index out of range
=
Done
  • We should have proper indentation in the SQL tab= once we give the parameters. Currently it looks like below for "Optio= ns" value.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 CREATE FOREIGN TABLE "1_test"."5_test"
= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ()
=C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SERVER asas
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 OPTIONS (test1 'test2'
=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 , test3 'test4');
Done
  • If user provide foreign table name and do not pro= vide foreign server and click on SQL tab then we are getting error on brows= er side as below. We should have proper error handling in this case.
    • =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Couldn't f= ind the required parameter (ftsrvname).
Done
  • Create = the foreign table, add the constraint and do not provide any constraint inf= ormation then SQL generated is wrong.
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0CREATE FOREIGN TABLE "1_test"."9_tes= t"
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0()
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0SERVER test_fsrv;
=

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ALTER FOREIGN TABLE = "1_test"."9_test"
=C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 ADD CONSTRAINT None CHECK ();
=
Done
  • When we click on the foreign table collection node then "Comment&= quot; column is blank even though we have comment in the foreign table.
    • Done
    • Create the foreign table on PG 9.1 and after pressin= g Save button we are getting below error.
    =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "the JSON object must be str, n= ot 'list'"
    Done
    • Whe= n we delete the options parameters then it gives SQL error because DROP sta= tement does not include the value.
    =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 ALTER FOREIGN TABLE public.test_12
    = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 OPTIONS ( DR= OP ser2 'val2');
    Done
    =
    • There are some new fun= ctionality added in PG 9.5. Do we really need to implement ? Need to discus= s with Dave/Ashesh. Below are the new functionality.
    =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0- In create foreign table,w= e have added column constraint but "table constraint" is added fr= om 9.5.Do we really requir= e to add table constraint ?
    =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0- In alter foreign table, below are the new functio= nality added.
    =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A01.=C2=A0 ALTER [ COLUMN ] column_name SET STORAG= E { PLAIN | EXTERNAL | EXTENDED | MAIN }
    =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A02.=C2=A0 DISABLE TRIGGE= R [ trigger_name | ALL | USER ]
    =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A03.=C2=A0 ENABLE TRIGGER [ trig= ger_name | ALL | USER ]
    =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A04.=C2=A0 ENABLE REPLICA TRIGGER trigger_= name
    =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A05.=C2=A0 ENABLE ALWAYS TRIGGER trigger_name
    =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A06.= =C2=A0 SET WITH OIDS
    =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A07.=C2=A0 SET WITHOUT OIDS

    <= /div>
    As per the discussion, we will add these funct= ionality into the next phase.
    <= div>
    Do let us know in case of any queries.

    <= div>Thanks,
    Neel Patel

    =
    On Tue, Apr 5, 2016 at 2:27 PM, Khushboo V= ashi <khushboo.vashi@enterprisedb.com> wrote:<= br>
    Hi,

    Pl= ease find updated patch for the Foreign Table Module.

    <= div>This patch is dependent on=C2=A0
    1. Backgrid Depscell Patch, = (submitted by me)
    2. NodeAjaxOptionsCell Transform change patch, = on which Ashesh and Murtuza are working

    Thanks,
    Khushboo




    On Wed, F= eb 24, 2016 at 2:57 PM, Khushboo Vashi <khushboo.vashi@enter= prisedb.com> wrote:
    Hi,

    I have updated the Foreign Table module as= below:

    - Used 'NodeByListControl' to get s= chemas, in the foreign_table.js file as suggested by Ashesh to avoid code r= edundancy.

    - Applied 'Security Label Macro'=C2= =A0 Patch (Implemented by Harshal).
    =C2=A0 To test the Foreign Table p= atch, 'Security Label Macro' patch must be applied first as that is= not committed yet.

    Please find attached Forei= gn Table Patch.

    Thanks,
    Khushboo
    =

    On Tue, Feb 23, 2= 016 at 6:53 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.= com> wrote:
    Hi,

    P= lease find attached patch for the Foreign Table Module.

    The patch wi= ll be modified after Types module implementation as we need to populate Bas= e Type=C2=A0 and some Type related validations from the Types module.
    Please review it and let me know the feedback.

    Thanks,
    Khushboo=




    --
    Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
    To make changes to your subscription:
    http://www.postgresql.org/mailpref/pgadmin-hackers=



    <pgAdmin4_Foreign_tables_ver2.patch>
    --
    Sent via = pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
    To make chang= es to your subscription:
    http://www.postgresql.org/mailpref/pgadmi= n-hackers





    --
    Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
    To make changes to your subscription:
    http://www.postgresql.org/mailpref/pgadmin-hackers=


    --047d7bfe9abacbdd55053267e160--