Re: patch for cast module

Hi Akshay,

PFA the updated patch.

Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Thu, Feb 11, 2016 at 12:09 PM, Akshay Joshi <
akshay(dot)joshi(at)enterprisedb(dot)com> wrote:

> Hi Sanket
>
> Most of the review comments has been resolved but I found some issues with
> this patch
>
> - When select some system cast it is showing wrong query, for example
> when select "abstime->date" in pgAdmin3 it is showing "AS ASSIGNMENT"
> and with your code it is showing "AS IMPLICIT".
> - For some of the system casts SQL query showing "WITH FUNCTION date"
> instead of "WITH FUNCTION date(abstime)" source type is not appended
> in query with new code.
> - For casts "bit->"bit"" function and target type is not listed.
> - When we create a new cast like "character->bytea" without selecting
> function, it creates successfully but when we select the newly created cast
> it shows the function "bpcharsend" in functions property. It may come
> for other combinations too, please verify.
> - Please fixed warnings in python file by using pep8 tool.
>
>
> On Mon, Feb 8, 2016 at 3:45 PM, Sanket Mehta <sanket(dot)mehta(at)enterprisedb
> .com> wrote:
>
>> Hi Akshay,
>>
>> PFA the revised patch.
>> All the comments are inline.
>>
>>
>> Regards,
>> Sanket Mehta
>> Sr Software engineer
>> Enterprisedb
>>
>> On Fri, Feb 5, 2016 at 12:43 PM, Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Sanket
>>>
>>> Below are the review comments
>>>
>>> - As "Show System Object" is not implemented yet, we should show all
>>> the objects by default.
>>>
>>> Done
>>
>>>
>>> - 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.
>>>
>>> Done.. Owner field is ignore as it is not a part of cast 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.
>>>
>>> resolved
>>
>>>
>>> - If node is leaf node then it should not show (+) expand symbol.
>>>
>>> Done
>>
>>>
>>> - Remove extra lines from create.sql and update.sql files as it
>>> shown in the SQL tab as well.
>>>
>>> Ignored as it was suggested by Ashesh.
>>
>>>
>>> - When select any system cast it is not showing function in the
>>> function control.
>>>
>>> Resolved.
>>
>>>
>>> - If comment is already exist and we remove the comments, sql query
>>> not generated in the SQL tab while it is generating in pgAdmin3.
>>>
>>> Done.
>>
>>
>>> *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?
>>>
>> After consulting with db server team, it is clear that they do not
>> maintain any mapping for compatible source and target types. in postgresql,
>> they pick selected source and target type and check them for compatibility.
>> So its not possible to filter out target type based on selected source type.
>>
>>>
>>>
>>>
>>> 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*
>>>
>>
>>
>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>