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
>
>

Reply via email to