Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jLQwZ-0000Fx-7w for pgadmin-hackers@arkaria.postgresql.org; Mon, 06 Apr 2020 12:33:35 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1jLQwY-000170-34 for pgadmin-hackers@arkaria.postgresql.org; Mon, 06 Apr 2020 12:33:34 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jLQwX-00016t-KP for pgadmin-hackers@lists.postgresql.org; Mon, 06 Apr 2020 12:33:33 +0000 Received: from mail-io1-xd34.google.com ([2607:f8b0:4864:20::d34]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jLQwU-0007lu-En for pgadmin-hackers@postgresql.org; Mon, 06 Apr 2020 12:33:32 +0000 Received: by mail-io1-xd34.google.com with SMTP id y17so13456279iow.9 for ; Mon, 06 Apr 2020 05:33:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VRFm55YYE85QcDOMppHaFM7cq/PuTZx1vvsfigMLvI8=; b=Azf+5S6zjrrE0Gqnh/rF1HD8zDJ7xhrCVOk8mk1zZLbZVls3AUD65lM84wUdzhIdiK Gh6rrZy4Dz1gs57k4xgqu2KtlMx3oU917BBLVuu3kgEVzAbFfpFq3XAm+iLOBR0xlhdO GCxM2nXoYgWMsglVgob5dR56kJehjmm22a6qlArW4lHY6pOeC5VYNRY22G9kTkqIaqzG xhcQIYRO/LmbUXdbx6peZMR67nd+fDbUpzzInYmr9guiQnovWcioHKmrUkG2pul0a6Tx WKK8nqq1ZBDcY6aoz8Obsggug/TWcZkBetkhr9v6iYqPxx2SgxHp7UiwayzegU2Jpnxr S+PA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VRFm55YYE85QcDOMppHaFM7cq/PuTZx1vvsfigMLvI8=; b=UxSuRlhZYEjTCwgRShcd8uzqcWFwmzt1SBPLVMFr6HhlO9a7GyhwLSgKqNw4awMb9M kKWqwG2kvS/WtRarQFxRLkJGZUNR3M7mtaC2qNfiZCR1+l29WH/r8pVJnYM7M95YJfdi yuChQBlcsoY63YD+VOMJ1umErpMqYNNHMgLMTNJStwonvHB7+HWIZRMZjSd+wrWMc89J 32EDx6PM20k4g5NSHPczSGzfEbaeYLPfYyWL7HvUjyLJB5WqZKzOepVWU/k88OS14kTB 83710dpRUsP6Lb3v7wsiA/GImuRumcIgQWfZej28XQWdCHJnAaEIkYxGc39e81Lb1Pvr WAOA== X-Gm-Message-State: AGi0PuZm4CB2bzm+GNgYPhPu5Mm0/NPDoN0wK0qMZDLQTmf5OR+DO9uR nxJt/9YSeDGs7GXSp2mgL3stMhTnjzIMRxmxm+2z4aQ3hdgASO/2WFmRlOSKKH0S1d5NPrBkNKX KHzVwQdz5xijdqcFsZapIB13XOjIXVlwLNZWFHXiQyf2lOsToKmYWnqGCvesDB+4wa30sb17Qts ikFEr7s4m5DHD7xg9hjHtlTPg2OiQvb1Wt/IlKez1u6f4FP84rjPk= X-Google-Smtp-Source: APiQypKsJ2eD3wE40CKrpWL5rq+QPJjg4sjgoMopiRjP92zvfIlIhnBvyxkiAnQrrzNvWAear7cjsND5+wACxarZQh4= X-Received: by 2002:a5d:9911:: with SMTP id x17mr5317257iol.52.1586176409231; Mon, 06 Apr 2020 05:33:29 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Mon, 6 Apr 2020 18:03:18 +0530 Message-ID: Subject: Re: [pgAdmin][RM2172] Search Objects Functionality To: Aditya Toshniwal Cc: pgadmin-hackers , Khushboo Vashi Content-Type: multipart/alternative; boundary="0000000000006c7f7705a29e79ea" X-CLOUD-SEC-AV-Info: edb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000006c7f7705a29e79ea Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks, patch applied. Fixed some minor issues. On Mon, Apr 6, 2020 at 3:52 PM Aditya Toshniwal < aditya.toshniwal@enterprisedb.com> wrote: > Hi Hackers, > > Please find the attached rebased patch. > > On Mon, Apr 6, 2020 at 3:22 PM Khushboo Vashi < > khushboo.vashi@enterprisedb.com> wrote: > >> Hi Aditya, >> >> Please resend the rebased patch, it does not apply. >> >> Thanks, >> Khushboo >> >> On Fri, Apr 3, 2020 at 2:44 PM Aditya Toshniwal < >> aditya.toshniwal@enterprisedb.com> wrote: >> >>> Hi Hackers, >>> >>> Attached is the updated patch. >>> With this, >>> 1) I've displayed the rows count detail at the bottom of the dialog. >>> This will help in both cases, when there are rows and when there are no= ne. >>> 2) As discussed, a user can now apply object types dropdown filter on >>> already loaded data. >>> 3) I've not made changes for the multilevel partition icon because it >>> would be too much to do for an icon. We're already showing the type nam= e in >>> the grid. Adding extra SQL joins and making the query slower for the ic= on >>> is not desirable. >>> 4) Fixed some gettext issues as mentioned in the review. >>> >>> Please review. >>> >>> >>> On Thu, Apr 2, 2020 at 5:54 PM Aditya Toshniwal < >>> aditya.toshniwal@enterprisedb.com> wrote: >>> >>>> Hi Khushboo, >>>> >>>> On Thu, Apr 2, 2020 at 4:49 PM Khushboo Vashi < >>>> khushboo.vashi@enterprisedb.com> wrote: >>>> >>>>> >>>>> >>>>> On Thu, Apr 2, 2020 at 4:30 PM Aditya Toshniwal < >>>>> aditya.toshniwal@enterprisedb.com> wrote: >>>>> >>>>>> Hi Khushboo, >>>>>> >>>>>> Thank you for reviewing. >>>>>> >>>>>> >>>>>> On Thu, Apr 2, 2020 at 4:09 PM Khushboo Vashi < >>>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Aditya, >>>>>>> >>>>>>> Review comments: >>>>>>> >>>>>>> *UI:* >>>>>>> >>>>>>> 1. When no object is found, the default message should be given, >>>>>>> currently no message displayed. >>>>>>> 2. Can we have a tooltip on the row "Double click to locate the >>>>>>> object in the browser" ? >>>>>>> 3. Full stop is missing in the message column objects are disabled >>>>>>> in the browser. You can enable them in the preferences dialog ( :D = ) >>>>>>> and also, we should start the statement with the capital letter. >>>>>>> 4. If possible, use the multilevel partition table symbol same as >>>>>>> the browser tree. >>>>>>> 5. gettext is missing from the search grid header. >>>>>>> >>>>>> I'll fix all above. >>>>>> >>>>>>> 6. Suggestion: The search button should be at the end (after type >>>>>>> combobox). The current position of the controls suggest that searc= h for >>>>>>> the objects and then filter it out but that's not the case. >>>>>>> >>>>>> I've actually kept the most frequently used controls together. The >>>>>> probability of using the types filter is less and a user would gener= ally go >>>>>> for full search. This is how even we generally do. We search first a= nd then >>>>>> apply filter if required >>>>>> >>>>> Right, so type based search on slickgrid data would be useful. >>>>> >>>> =F0=9F=91=8D >>>> >>>>> After changing the type, we have to click on the search button. >>>>>>> In the current positioning, we should fetch all the records from th= e >>>>>>> backend and then filter those out depending on the type at the clie= nt side >>>>>>> only, so that will reduce the server requests and slickgrid is effi= cient it >>>>>>> do so. >>>>>>> >>>>>> I'll look into this. My only concern is the data may be outdated, bu= t >>>>>> I agree to filter in slickgrid on type change. The user can hit sear= ch >>>>>> again if required. >>>>>> >>>>> >>>>>>> *Backend:* >>>>>>> >>>>>>> 1. We do have the list of blueprint, so we can use that list instea= d >>>>>>> of taking the hard coe list in the init method of SearchObjectsHelp= er >>>>>>> class. >>>>>>> >>>>>> The reason is, we do not support all objects for search objects. Onl= y >>>>>> objects under a database are supported. The probability of node type= change >>>>>> is very less. >>>>>> >>>>> True but we can maintain the skip list (which would be less) and we d= o >>>>> have bluprint start with NODE, so it will be easier to fetch. >>>>> >>>> I would prefer the "in" list rather than "skip" list. Each time a new >>>> node is added to pgAdmin, we will have to update the skip list in >>>> search objects. With the "in" list, search objects has better control. >>>> >>>>> 2. While searching the object, we create an object of SearchObjectsHe= lper >>>>>>> on each request. We can create it once while initializing and utili= ze it on >>>>>>> every search. >>>>>>> >>>>>> The intention is to keep SearchObjectsHelper stateless. The object >>>>>> is created based on the request data and it is easier to maintain >>>>>> independently. >>>>>> >>>>>>> >>>>>>> Note: The functionality is working fine. >>>>>>> >>>>>> Great. Thanks. >>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> Khushboo >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Thu, Apr 2, 2020 at 9:31 AM Khushboo Vashi < >>>>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi < >>>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> Hi Khushboo, >>>>>>>>> >>>>>>>>> Can you please review it. >>>>>>>>> >>>>>>>> I am on it. >>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Mar 30, 2020 at 2:39 PM Aditya Toshniwal < >>>>>>>>> aditya.toshniwal@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> Hi Hackers, >>>>>>>>>> >>>>>>>>>> Attached is the patch to implement search objects functionality >>>>>>>>>> in pgadmin. >>>>>>>>>> The feature will allow a user to search for any object in a >>>>>>>>>> database. >>>>>>>>>> Highlights of the feature: >>>>>>>>>> 1) Search any object with user input text with at least 3 >>>>>>>>>> characters. >>>>>>>>>> 2) Search can be done on a specific object type by selecting fro= m >>>>>>>>>> the types dropdown. >>>>>>>>>> 3) The search results grid will show object name, object type an= d >>>>>>>>>> the object path on the browser tree. On double clicking the reco= rd, it will >>>>>>>>>> locate that object on the browser tree. The columns object name = and type >>>>>>>>>> are sortable. >>>>>>>>>> 4) The object nodes which are disabled (hidden) using preference= s >>>>>>>>>> will not be visible in the types dropdown. However, in the case = of all >>>>>>>>>> types, the search records will be visible for those types greyed= out. >>>>>>>>>> 5) You can also access search objects dialog using the button on >>>>>>>>>> the browser toolbar. >>>>>>>>>> >>>>>>>>>> Python and JS test cases added. Docs updated. >>>>>>>>>> Please review. >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Thanks and Regards, >>>>>>>>>> Aditya Toshniwal >>>>>>>>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | >>>>>>>>>> Pune >>>>>>>>>> "Don't Complain about Heat, Plant a TREE" >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> *Thanks & Regards* >>>>>>>>> *Akshay Joshi* >>>>>>>>> >>>>>>>>> *Sr. Software Architect* >>>>>>>>> *EnterpriseDB Software India Private Limited* >>>>>>>>> *Mobile: +91 976-788-8246* >>>>>>>>> >>>>>>>> >>>>>> >>>>>> -- >>>>>> Thanks and Regards, >>>>>> Aditya Toshniwal >>>>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune >>>>>> "Don't Complain about Heat, Plant a TREE" >>>>>> >>>>> >>>> >>>> -- >>>> Thanks and Regards, >>>> Aditya Toshniwal >>>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune >>>> "Don't Complain about Heat, Plant a TREE" >>>> >>> >>> >>> -- >>> Thanks and Regards, >>> Aditya Toshniwal >>> pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune >>> "Don't Complain about Heat, Plant a TREE" >>> >> > > -- > Thanks and Regards, > Aditya Toshniwal > pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune > "Don't Complain about Heat, Plant a TREE" > --=20 *Thanks & Regards* *Akshay Joshi* *Sr. Software Architect* *EnterpriseDB Software India Private Limited* *Mobile: +91 976-788-8246* --0000000000006c7f7705a29e79ea Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks, patch applied. Fixed some minor issues.=C2=A0
=

On Mon, Apr 6, 2020 at 3:52 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Hackers,

Please=C2=A0find the attached rebased patch.

On Mon, Apr 6, 2020 at 3:22 PM Khushboo Vashi <khushboo.vashi@enterprisedb.c= om> wrote:
Hi Aditya,

Please resend the rebased = patch, it does not apply.

Thanks,
Khushb= oo

On Fri, Apr 3, 2020 at 2:44 PM Aditya Toshniwal <aditya.toshniwal@en= terprisedb.com> wrote:
Hi Hackers,

Attached is the updated patch.
W= ith this,
1) I've displayed the rows count detail at the bottom=C2=A0of= the dialog. This will help in both=C2=A0cases, when there are rows and whe= n there are none.
2) As discussed, a=C2=A0user can now apply object=C2=A0ty= pes dropdown filter on already loaded data.
3) I've not made changes fo= r the multilevel partition icon because it would be too much to do for an i= con. We're already showing the type name in the grid. Adding extra SQL = joins and making the query slower for the icon is not desirable.
4) Fixed s= ome gettext issues as mentioned in the review.

Please review.

On Thu, Apr 2, 2020 at 5:54 PM Aditya Toshniwal <aditya.toshniw= al@enterprisedb.com> wrote:
Hi=C2=A0Khushboo,

On T= hu, Apr 2, 2020 at 4:49 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com= > wrote:


On Thu, Apr 2, 2020 at 4:30 PM Aditya Tos= hniwal <aditya.toshniwal@enterprisedb.com> wrote:
<= div style=3D"font-family:verdana,sans-serif">Hi Khushboo,

Thank you for reviewing.


On Thu, Apr 2= , 2020 at 4:09 PM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrot= e:
Hi Aditya,

Review comments:

UI:

1. Wh= en no object is found, the default message should be given, currently no me= ssage displayed.
2. Can w= e have a tooltip on the row "Double click to locate the object in the = browser" ?
3. Full s= top is missing in the message=C2=A0column objects are disabled in th= e browser. You can enable them in the preferences dialog ( :D )
<= font face=3D"arial, sans-serif">and also, we should start the statement wit= h the capital letter.
4. = If possible, use the multilevel partition table symbol same as the browser = tree.
5. gettext is missi= ng from the search grid header.
I'll fi= x all above.=C2=A0
6. Suggestion:= The search button should be at the end (after type combobox).=C2=A0 The cu= rrent position of the controls suggest that search for the objects and then= filter it out but that's not the case.
=
I've actually kept the most frequently used controls together. The pro= bability=C2=A0of using the types filter is less and a=C2=A0user would gener= ally go for full search. This is how even we generally do. We search first = and then apply filter if required
Right, so type based search on slickgrid data would be useful.=C2=A0
=
=F0=9F=91=8D=C2=A0
After changing=C2=A0the type= , we have to click on the search button.
In the current positioning, we should fetch all the records= from the backend and then filter those out depending on the type at the cl= ient side only, so that will reduce the server requests and slickgrid is ef= ficient it do so.
I'll look into this. My only concern is the da= ta may be outdated, but I agree to filter in slickgrid on type change. The = user can hit search again if required.
=
=

Backend:

=
1. We do have the list o= f blueprint, so we can use that list instead of taking the hard coe list in= the init method of=C2=A0SearchObjectsHelp= er class.
The reason is, we do not s= upport all objects for search objects. Only objects under a database are su= pported. The probability of node type change is very less.
True but we can maintain the skip list (which wou= ld be less) and we do have bluprint start with NODE, so it will be easier t= o fetch.
I would prefer the "in" l= ist rather than "skip" list. Each time a new node is added to pgA= dmin, we=C2=A0will have to update the skip list in search objects.= With the "in" list, search objects has better control.
2. While searching the object, we create an ob= ject of=C2=A0SearchObjectsHelper on each request. We can create it o= nce while initializing and utilize it on every search.
The intention is to keep SearchObjectsHelper stateless. The object is created based on the re= quest data and it is easier to maintain independently.

Note:=C2=A0The funct= ionality is working fine.
Great. Thanks.<= /span>

Thanks,
Khushboo
<= div>




=
On Thu, Ap= r 2, 2020 at 9:31 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> w= rote:


On Wed, Apr 1, 2020 at 6:00 PM Akshay Joshi &= lt;aksha= y.joshi@enterprisedb.com> wrote:
Hi Khushboo,

Ca= n you please review it.
I am on it.=C2=A0

On Mon, Mar 30, 2020 at 2:39 P= M Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Hackers,

Attached is the patch to implement search objects functionality in= pgadmin.
The feature wi= ll allow a user to search for any object in a database.
Highlights=C2=A0of the feature:
1) Search any object with user inpu= t text with at least 3 characters.
2) Search can be done on a specific object type by selecting fro= m the types dropdown.
3)= The search results grid will show object name, object type and the object = path on the browser tree. On double clicking the record, it will locate tha= t object on the browser tree. The columns object name and type are sortable= .
4) The object node= s which are disabled (hidden) using preferences will not be visible in the = types dropdown. However, in the case of all types, the search records will = be visible for those types greyed out.
5) You can also access search=C2= =A0objects dialog using the button on the browser toolbar.

Python and JS test cases added. Docs updated.
Please review.

--
Thanks and Rega= rds,
Aditya Toshniwal
pgAdmin H= acker | Sr. Software Engineer |=C2=A0EnterpriseDB India |=C2=A0Pune
"Don't Compla= in about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
<= font color=3D"#3333FF">Sr. Software Architect=
<= font color=3D"#000000" face=3D"arial, sans-serif">EnterpriseDB Software = India Private Limited
Mobile: +91 976-788-8246=


--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer |=C2=A0Ent= erpriseDB India |=C2=A0Pune<= /span>
"Don't Complain about Heat, Plant a TREE"=


--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer |=C2=A0Ent= erpriseDB India |=C2=A0Pune<= /span>
"Don't Complain about Heat, Plant a TREE"=


--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer |=C2=A0Ent= erpriseDB India |=C2=A0Pune<= /span>
"Don't Complain about Heat, Plant a TREE"=


--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer |=C2=A0Ent= erpriseDB India |=C2=A0Pune<= /span>
"Don't Complain about Heat, Plant a TREE"=


--
Thanks & Regards
=
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
=
Mobile: += 91 976-788-8246
--0000000000006c7f7705a29e79ea--