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

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4] [Patch]: Foreign Table Module
Date: 2016-05-06 16:45:54
Message-ID: CAFOhELeXyKfYoR4TAA2h8XErhQ7f1X9xkykpmN+Q4C52WMkG=Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Fahar Abbas 2016-05-07 03:49:43 Re: New committer: Akshay Joshi
Previous Message Dave Page 2016-05-06 15:33:19 Re: Patch for pgAdmin4 package on Mac OS X