On Mon, May 9, 2016 at 5:50 PM, Khushboo Vashi < khushboo.va...@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.zabuaw...@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.enterprisedb.com/> *http://www.linkedin.com/in/asheshvashi* <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 abstime >> data type which have neither Length & Precision >> - It will not clear/ not allow user to clear old values & generates wrong >> 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 => "@Test#" ) & it >> breaks when we clicks on it. >> >> File >> "/home/murtuza/projects/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py", >> line 414, in properties >> data = self._fetch_properties(gid, sid, did, scid, foid) >> File >> "/home/murtuza/projects/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py", >> line 982, in _fetch_properties >> c['typlen'] = 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 = self._fetch_properties(gid, sid, did, scid, foid) >> File >> "/home/murtuza/projects/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/__init__.py", >> line 982, in _fetch_properties >> c['typlen'] = 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:* >> *=====================* >> >> *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 & Precision. >> >> >> Regards, >> Murtuza >> >> >> >> On 09-May-2016, at 10:43 am, Murtuza Zabuawala < >> murtuza.zabuaw...@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.va...@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.zabuaw...@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 = >>> Preferences(self.name <http://self.name/>, None) File >>> "/Users/murtuza/Desktop/pgadmin4/web/pgadmin/utils/preferences.py", line >>> 261, in __init__ db.session.commit()…..* >>> >>> *…..* >>> >>> * cursor.execute(statement, >>> parameters)sqlalchemy.exc.OperationalError: (OperationalError) database is >>> locked 'INSERT INTO module_preference (name) VALUES (?)' >>> ('NODE-foreign-table’,)* >>> >>> >>> 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. >>> >>> >>> <Screenshot from 2016-05-06 08-00-19.png><Screenshot from 2016-05-06 >>> 07-59-45.png><Screen Shot 2016-05-06 at 8.18.12 pm.png> >>> >>> >>> On 06-May-2016, at 3:55 pm, Khushboo Vashi < >>> khushboo.va...@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.pa...@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 tab. >>>> >>>> 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 NULL >>>> >>>> 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 proper >>>> validation in Column tab for all parameters. Currently if user do not >>>> 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 side 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 <http://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.va...@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.va...@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.va...@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 >>>>> >>>>> >>>> >>> <pgAdmin4_Foreign_tables_ver2.patch> >>> -- >>> 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 > >