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

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4] [Patch]: Foreign Table Module
Date: 2016-05-06 10:25:01
Message-ID: CAFOhELf010FXe8_1AsBcc_GxrLYD5QzpQft+URS6zwf=pxtCiA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

Attachment Content-Type Size
pgAdmin4_Foreign_tables_ver2.patch text/x-patch 106.1 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2016-05-06 10:39:34 [pgAdmin4][Patch]: TODO File
Previous Message Murtuza Zabuawala 2016-05-06 10:14:12 PATCH: Fixed typo in Server node