Re: [pgAdmin4] [Patch]: Foreign Table Module

On Mon, May 9, 2016 at 5:50 PM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)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(dot)zabuawala(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)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(dot)patel(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)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(dot)vashi(at)enterprisedb(dot)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(at)postgresql(dot)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(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>>
>>>
>>
>>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>