Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.80) (envelope-from ) id 1aRaaP-0002qy-2e for pgadmin-hackers@arkaria.postgresql.org; Fri, 05 Feb 2016 07:13:45 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84) (envelope-from ) id 1aRaaO-0000a4-LV for pgadmin-hackers@arkaria.postgresql.org; Fri, 05 Feb 2016 07:13:44 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84) (envelope-from ) id 1aRaaN-0000Zy-Ue for pgadmin-hackers@postgresql.org; Fri, 05 Feb 2016 07:13:44 +0000 Received: from mail-ob0-x235.google.com ([2607:f8b0:4003:c01::235]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84) (envelope-from ) id 1aRaaG-0006TF-Vg for pgadmin-hackers@postgresql.org; Fri, 05 Feb 2016 07:13:42 +0000 Received: by mail-ob0-x235.google.com with SMTP id is5so80836174obc.0 for ; Thu, 04 Feb 2016 23:13:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=Wt2a8bCkXghS8HUTte9PH68PmTdkFUr0X8T7HlMk2fA=; b=VZPOk4Si1Iuz3Y/6G/qlJ1hitlB3tf0ZQCRqTysmf760BlI+TzMJogKbkPgFXlZDJC ByERb7aae3mrl1JLQbicaawL9YZq+RnFYMlkEjxXn3yKDd6YF4NbHIJmRVnhZbQUc7KO cxZwN3ZY0kQbd+0Y6OLRSSCl1AXr0Zflr7zbwJD6m5HcqTVNPn3p/kRbO4ZhSfx7YlN9 4K+cr5agUwUNKYbkgNyTRlxhoJ4DWkebQ6uwPhjJA0Psagtu0wSCcM/w4B+41xFS6zIX nzX2f/vKQCfrn050DIl6pURswxNOio5fFU5OzhA5JfQOcGla2Vq3oFKE1TRYdfaTn6F8 ekVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=Wt2a8bCkXghS8HUTte9PH68PmTdkFUr0X8T7HlMk2fA=; b=f5y9P2GEEFPitrjAOxCa87gH1IxuSSPxPgEcEvrpYWv1jWc5sPOh7W+3cqy7mH4Z1C BKE901sWNqvBrdJwoHnHST+b5yrYqBd0hj2dTuGVkIlrcUZBhxiDwVbmnDaeEeiBHiNQ EQBcxi0k/Qk7ep/StzRYLOt9GJBRdlSjmUHqxXhU0hoE60vsM63a/MHmSBynS4TtCfFl 8IP8Ncu8HBNMVG4W8OPi7d+fytKNp4TAZxbNZMrRSgsdUBDzYAYMaRqWwM1YWVoRbPyz EU5UijNzCV6Y7RKzpnz5m6iL8ryVWjFr8u6ukGk/gdnJoqzv5BggFkp1KNjE/sO/kOBZ sfKw== X-Gm-Message-State: AG10YOQrEA7YRKaW3rYm0hoghzltCmZGNrwoUXt1BLCv+D8lrrFUDpZEzQoy98JoCKJW2JpdjIuR139ovQsA6btC MIME-Version: 1.0 X-Received: by 10.60.227.74 with SMTP id ry10mr10821648oec.48.1454656415655; Thu, 04 Feb 2016 23:13:35 -0800 (PST) Received: by 10.202.59.197 with HTTP; Thu, 4 Feb 2016 23:13:35 -0800 (PST) In-Reply-To: References: Date: Fri, 5 Feb 2016 12:43:35 +0530 Message-ID: Subject: Re: patch for cast module From: Akshay Joshi To: Sanket Mehta Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=001a1136935eed05c0052b00968d X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a1136935eed05c0052b00968d Content-Type: text/plain; charset=UTF-8 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 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.mehta@enterprisedb.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 > > 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.mehta@enterprisedb.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.mehta@enterprisedb.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@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 > > -- *Akshay Joshi* *Principal Software Engineer * *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* --001a1136935eed05c0052b00968d Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Sanket<= /span>=C2=A0

Below are the review comments
  • As "Show System Object" is not implemented yet, we should sh= ow 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 onl= y one control "Comment" can that be a part of the Definition tab?= ??
  • For some data type like "Character", "Integer&quo= t;, it is throwing error that data type doesn't exist.
  • If node = is leaf node then it should not show (+) expand symbol.
  • Remove extr= a lines from create.sql and update.sql f= iles 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 w= e remove the comments, sql<= /span> query not generated in the SQL tab while it is generating in pgAdmin3.
    • Ques= tion: With current implementation in "pgAdmin3" to create &qu= ot;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 imp= lement it like when user select the source type from combo box, target type= combo will only show types which are physically compatible?
    =
    =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0

    On Thu, Feb 4, 2016 a= t 6:31 PM, Sanket Mehta <sanket.mehta@enterprisedb.com>= wrote:
    Hi Akshay,

    PFA the latest patch for Cast module.
    Ple= ase 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.mehta@enterprisedb.com= > wrote:
    H= i Neel.

    PFA the revised patch which has changed accordin= g to your comments.
    Please do review it and let me know in case a= nything is missing.



    Regards,Sanket Mehta
    Sr Software engineer
    Enterprisedb
    <= /div>

    On Wed, Jan 20, 2016 at 10:= 20 AM, Neel Patel <neel.patel@enterprisedb.com> wr= ote:
    Hi Sanket,

    Below are the review comments.

    - = When we edit any existing cast node then it gives error "Response o= bject has no attribute strip". This error is coming because genera= ted SQL is=C2=A0
    =C2=A0 wrong.
    - Unnecessary debug logs are coming = on console. Please remove unnecessary debug logs.
    - In some of th= e sql file, 'qtIdent' and 'qtLiteral' is not used. Please c= heck all the SQL files.
    - "Delete" cast functiona= lity is not working. Error is getting displayed saying "syntax erro= r at or near "castsource".=C2=A0
    -=C2=A0"Delet= e cascade" functionality is not working=C2=A0- error is getting displa= yed saying "The requested URL not found".
    - Do t= he 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 . T= est the basic functionality of "create", "Edit" and &qu= ot;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.mehta@enterprisedb.com> wrote:
    Hi,

    PFA updat= ed patch for cast module as per check list provided by Neel.
    Plea= se do review it and let me know in case of anything is missing.
    <= br>


    Regards,
    Sanket Mehta
    Sr Sof= tware engineer
    Enterprisedb

    On Mon, Jan 18, 2016 at 7:1= 6 PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote:
    Hi,

    PFA patch for cast module.
    Please do review it and let m= e know in case of any issue.


    Regards,
    Sanket Mehta
    Sr Software eng= ineer
    Enterprisedb



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




    --
    Akshay Joshi
    Principal Software Engineer=C2= =A0


    =
    Pho= ne: +91 20-3058-9517
    Mobile: +91 976-788-8246
    --001a1136935eed05c0052b00968d--