Re: patch for cast module

Hi Sanket

Below are the review comments

- As "Show System Object" is not implemented yet, we should show all the
objects by default.
- As in pgAdmin3 when click on Casts (Collection) node it should show
only Name, Owner and Comments. With current code it is showing all the
properties.
- Properties Tab contains only one control "Comment" can that be a part
of the Definition tab???
- For some data type like "Character", "Integer", it is throwing error
that data type doesn't exist.
- If node is leaf node then it should not show (+) expand symbol.
- Remove extra lines from create.sql and update.sql files as it shown in
the SQL tab as well.
- When select any system cast it is not showing function in the function
control.
- If comment is already exist and we remove the comments, sql query not
generated in the SQL tab while it is generating in pgAdmin3.

*Question*: With current implementation in "pgAdmin3" to create "Cast" user
will have to select source type and target type and then click on OK
button. If source and target type is not physically compatible, server will
throw an error. I am not sure, but instead of that can we implement it like
when user select the source type from combo box, target type combo will
only show types which are physically compatible?

On Thu, Feb 4, 2016 at 6:31 PM, Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>
wrote:

> Hi Akshay,
>
> PFA the latest patch for Cast module.
> Please do review it and let me know if anything is missing.
>
>
> Regards,
> Sanket Mehta
> Sr Software engineer
> Enterprisedb
>
> On Wed, Jan 20, 2016 at 5:03 PM, Sanket Mehta <
> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>
>> Hi Neel.
>>
>> PFA the revised patch which has changed according to your comments.
>> Please do review it and let me know in case anything is missing.
>>
>>
>>
>> Regards,
>> Sanket Mehta
>> Sr Software engineer
>> Enterprisedb
>>
>> On Wed, Jan 20, 2016 at 10:20 AM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com
>> > wrote:
>>
>>> Hi Sanket,
>>>
>>> Below are the review comments.
>>>
>>> - When we edit any existing cast node then it gives error "*Response
>>> object has no attribute strip*". This error is coming because generated
>>> SQL is
>>> wrong.
>>> - Unnecessary debug logs are coming on console. Please remove
>>> unnecessary debug logs.
>>> - In some of the sql file, 'qtIdent' and 'qtLiteral' is not used. Please
>>> check all the SQL files.
>>> - "Delete" cast functionality is not working. Error is getting displayed
>>> saying *"syntax error at or near "castsource"*.
>>> - "Delete cascade" functionality is not working - error is getting
>>> displayed saying *"The requested URL not found".*
>>> - Do the proper comments, in some of the function like "script_load" ,
>>> comments are wrong.
>>> - Is "configs" really required in __init__.py file ? We have not seen
>>> any usage for this. Please remove it if it is not required.
>>> - Remove commented code from the source file.
>>>
>>> Please check all the generated SQL statements . Test the basic
>>> functionality of "create", "Edit" and "Delete" node before sending patch
>>> file.
>>>
>>> Do let us know for any comments/issues.
>>>
>>> Thanks,
>>> Neel Patel
>>>
>>> On Tue, Jan 19, 2016 at 8:06 PM, Sanket Mehta <
>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi,
>>>>
>>>> PFA updated patch for cast module as per check list provided by Neel.
>>>> Please do review it and let me know in case of anything is missing.
>>>>
>>>>
>>>>
>>>> Regards,
>>>> Sanket Mehta
>>>> Sr Software engineer
>>>> Enterprisedb
>>>>
>>>> On Mon, Jan 18, 2016 at 7:16 PM, Sanket Mehta <
>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> PFA patch for cast module.
>>>>> Please do review it and let me know in case of any issue.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Sanket Mehta
>>>>> Sr Software engineer
>>>>> Enterprisedb
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> 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
>
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*