Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dL9FT-00049S-8j for pgadmin-hackers@arkaria.postgresql.org; Wed, 14 Jun 2017 14:26:19 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1dL9FS-0003BH-9I for pgadmin-hackers@arkaria.postgresql.org; Wed, 14 Jun 2017 14:26:18 +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_2) (envelope-from ) id 1dL9FC-0002lA-6c for pgadmin-hackers@postgresql.org; Wed, 14 Jun 2017 14:26:02 +0000 Received: from mail-wr0-x231.google.com ([2a00:1450:400c:c0c::231]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1dL9F8-0006VY-EY for pgadmin-hackers@postgresql.org; Wed, 14 Jun 2017 14:26:00 +0000 Received: by mail-wr0-x231.google.com with SMTP id 36so2857648wry.3 for ; Wed, 14 Jun 2017 07:25:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=fT0bSrILuCgRBEc8Hjki/iZQayERU6zYCKZAH4q9//A=; b=IuE1TmJGv+weH1vOhrgr4iXNUo7cay4kluPZD1RYcaJ5M5BVS3eKAzUbceGk/5j5LZ k39FcLRFinDmQNFyptRnROq99QkcthuV/5i1riGTS5zBc/j8s6fgIzT8YYf4bnMEkrf1 1LO65x/xWGv4xrYhaX3k2OuN8P3FYtx93rs5kt9hgstkSlSrlP+vwGRbxPP4ZTA2Khe9 GMTU3C+W7sndpJn1r6Hwi1GNkBDy1bJX0mQo7/TQmeM2C+xWlTyemNJxYA/REdgwXi9U W3kDpmxeR7lfj4G9poWBCINDHro/rd+5bbsqHGNFcRHJFTJtrvXeGMHFq18vd+533zM0 XVgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fT0bSrILuCgRBEc8Hjki/iZQayERU6zYCKZAH4q9//A=; b=fm67d7VmAn88sEGPBxZiPCJwfM9TY+wr6s3tPLf5cefLfy0SfYfqcuWsKpTiTDeIkQ H7RyfopFPbCZET3xWB+XCAhPmavnfMmv0YotkgeQr/kxAa35O/RYrCyPJ/R7x44yvokP FK7Hb8XrrEaXu0Q9uvAEBN5jven+F55/RxJdhNzHvbfHWon6OioS/enAOMzkivJNgNtp k7pltFgjEOISZmloJfTZOmjquJ+F/dxPRoW5EC/diRIPtB5uL9Iy1r0AL+sw2Q87NKzb pKnuEzhbuCKDQk2loLLQUkVAd4cshRfBwF7K25T01urOpOd7VCcDGxTUpK9n6lt3o2NK EoNQ== X-Gm-Message-State: AKS2vOwJn8S/965srNN4y2/JGG4AqI9V1fRtp9kQ71PFuiFdhrh2GSr6 uZU6J5AQjwytfg7OAYYVkhqrC3tsnw== X-Received: by 10.223.154.141 with SMTP id a13mr220027wrc.139.1497450356628; Wed, 14 Jun 2017 07:25:56 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.71.203 with HTTP; Wed, 14 Jun 2017 07:25:36 -0700 (PDT) In-Reply-To: References: From: Harshal Dhumal Date: Wed, 14 Jun 2017 19:55:36 +0530 Message-ID: Subject: Re: Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4] To: Ashesh Vashi Cc: Dave Page , Harshal Dhumal , pgadmin-hackers Content-Type: multipart/alternative; boundary="f403045f523a93636b0551ec54b5" 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 --f403045f523a93636b0551ec54b5 Content-Type: text/plain; charset="UTF-8" On Wed, Jun 14, 2017 at 6:21 PM, Ashesh Vashi wrote: > On Wed, Jun 14, 2017 at 6:19 PM, Dave Page wrote: > >> Hi, >> >> Sorry - it's drifted out again, I suspect because of the work Ashesh >> has been doing. Can you rebase please? Check with Ashesh first though, >> in case he's about ready to commit another big change. >> > I am not. :-) > > Sure, I'll send updated patch. > -- > > Thanks & Regards, > > Ashesh Vashi > EnterpriseDB INDIA: Enterprise PostgreSQL Company > > > > *http://www.linkedin.com/in/asheshvashi* > > >> >> Thanks. >> >> On Fri, Jun 9, 2017 at 10:08 AM, Harshal Dhumal >> wrote: >> > Hi, >> > >> > >> > Please find rebased patch >> > >> > -- >> > Harshal Dhumal >> > Sr. Software Engineer >> > >> > EnterpriseDB India: http://www.enterprisedb.com >> > The Enterprise PostgreSQL Company >> > >> > On Thu, Jun 8, 2017 at 6:40 PM, Harshal Dhumal >> > wrote: >> >> >> >> Ignore this patch. >> >> Rebase and migration of feature tests and jasmine tests required. >> >> >> >> -- >> >> Harshal Dhumal >> >> Sr. Software Engineer >> >> >> >> EnterpriseDB India: http://www.enterprisedb.com >> >> The Enterprise PostgreSQL Company >> >> >> >> On Thu, Jun 8, 2017 at 3:56 PM, Harshal Dhumal >> >> wrote: >> >>> >> >>> Hi, >> >>> Please find attached updated patch for feature RM2137. >> >>> >> >>> Changes in this patch: >> >>> 1. Patch rebased. >> >>> >> >>> 2. Updated existing feature tests which requires changes due to this >> >>> feature. >> >>> affected feature test cases: >> >>> i. PGDataypeFeatureTest >> >>> ii. CheckForXssFeatureTest >> >>> >> >>> 3. Updated existing jasmine test cases which requires changes due to >> this >> >>> feature. >> >>> affected jasmine test cases: >> >>> i. copy data >> >>> ii. range_boundary_navigator >> >>> iii. row_selector >> >>> iv. set_stages_rows >> >>> >> >>> 4. New feature tests added >> >>> i. on demand result set on scrolling. >> >>> ii. on demand result set on grid select all. >> >>> iii. on demand result set on column select all. >> >>> iv. explain query >> >>> v. explain query with verbose >> >>> vi. explain query with costs >> >>> vii. explain analyze query >> >>> viii. explain analyze query with buffers >> >>> ix. explain analyze query with timing >> >>> x. auto commit disabled. >> >>> xi. auto commit enabled. >> >>> xii. auto rollback enabled. >> >>> xiii. cancel query. >> >>> >> >>> >> >>> >> >>> -- >> >>> Harshal Dhumal >> >>> Sr. Software Engineer >> >>> >> >>> EnterpriseDB India: http://www.enterprisedb.com >> >>> The Enterprise PostgreSQL Company >> >>> >> >>> On Tue, May 16, 2017 at 8:14 PM, Dave Page wrote: >> >>>> >> >>>> >> >>>> >> >>>> On Mon, May 15, 2017 at 7:40 PM, Harshal Dhumal >> >>>> wrote: >> >>>>> >> >>>>> Hi, >> >>>>> >> >>>>> On Sat, May 13, 2017 at 12:35 AM, Joao Pedro De Almeida Pereira >> >>>>> wrote: >> >>>>>> >> >>>>>> We were only able to apply the patch on 1f903ba2 (were seeing patch >> >>>>>> does not apply due to sqleditor.js conflicts) >> >>>>>> The javascript tests passed, but we were unable to copy rows or >> >>>>>> columns or cells when running the application. Could you run >> feature tests? >> >>>>> >> >>>>> There are three modes sqleditor can be launched >> >>>>> 1. Query tool (Tools menus -> Query Tool) >> >>>>> 2. Datagrid. (Right click on any table/view -> View Data -> View >> >>>>> All/First 100/Last 100/Filtered rows) >> >>>>> 3. Scripts (Right click on any table/view -> >> >>>>> INSERT/CREATE/UPDATE/DELETE/SELECT) >> >>>>> >> >>>>> Paste functionality is only enabled in Datagrid and table has >> Primary >> >>>>> key otherwise it's disabled. In your case row might have been >> copied but you >> >>>>> were unable paste because you might be trying to paste the rows in >> Query >> >>>>> tool. Please try again in Datagrid with table having Primary key. >> >>>>> >> >>>>>> >> >>>>>> >> >>>>>> Now that more functionality is being added to sqleditor.js, this >> may >> >>>>>> be a good time to extract the functionality to separate files. >> This will >> >>>>>> increase readability, and encourage separation of concerns. It >> will also >> >>>>>> make changes easier to test in isolation. >> >>>>> >> >>>>> Ok. Let me check if I can separate out ant functionalities. >> >>>>> >> >>>>>> >> >>>>>> It's probably a good idea to test the changes made to the python as >> >>>>>> well as javascript code. In this case, the new behavior of poll() >> in >> >>>>>> sqleditor __init__ should be tested. >> >>>>> >> >>>>> >> >>>>> At this point we don't have any python unit tests that can test >> >>>>> sqleditor backend (python code). >> >>>>> @Dave should I include python unit test cases in this patch? >> >>>> >> >>>> >> >>>> We do have some feature tests that hit the query tool - Murtuza wrote >> >>>> some anti-XSS validation tests for example, and Khushboo has been >> working on >> >>>> some datatype rendering tests. >> >>>> >> >>>> As a general rule, I prefer we focus more on feature tests now than >> the >> >>>> API tests - they cover the whole app end-to-end of course. The >> disadvantages >> >>>> are: >> >>>> >> >>>> - The treeview isn't reliable enough for me to enable those tests on >> the >> >>>> CI server yet. >> >>>> >> >>>> - They can take a long time to run, so we need to test multiple >> things >> >>>> at once wherever possible. That means minimising browser reloads, or >> new >> >>>> instances of tools like the Query Tool - or even the number of >> queries >> >>>> executed through the query tool as part of a test. >> >>>> >> >>>> That said, yes, if there are specific things that are not covered by >> >>>> Murtuza and Khushboo's work, we should test them. For example, >> loading all >> >>>> rows when the user selects all, running/rendering EXPLAIN, >> auto-commit vs. >> >>>> auto-rollback (and combinations thereof). >> >>>> >> >>>> The standard moving forwards should be to include feature tests for >> new >> >>>> functionality and Jasmine tests for algorithmic JS code. >> >>>> >> >>>> I also agree with Joao on the modularisation of JS code. Testable and >> >>>> reusable code should be in "library" files, and we should work to >> minimise >> >>>> the amount of JS templates - for the most part, that means moving to >> the >> >>>> client-side translation mechanism which Tira worked on, and I've >> done some >> >>>> early migration work. >> >>>> >> >>>> Thanks. >> >>>> >> >>>> >> >>>> -- >> >>>> Dave Page >> >>>> Blog: http://pgsnake.blogspot.com >> >>>> Twitter: @pgsnake >> >>>> >> >>>> EnterpriseDB UK: http://www.enterprisedb.com >> >>>> The Enterprise PostgreSQL Company >> >>> >> >>> >> >> >> > >> >> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> >> -- >> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgadmin-hackers >> > > --f403045f523a93636b0551ec54b5 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, Jun 14, 2017 at 6:21 PM, Ashesh Vashi <ashesh.vash= i@enterprisedb.com> wrote:
=
On Wed, Jun 14, 2017 at 6:19 PM, Dave Page &= lt;dpage@pgadmin.org= > wrote:
Hi,

Sorry - it's drifted out again, I suspect because of the work Ashesh has been doing. Can you rebase please? Check with Ashesh first though,
in case he's about ready to commit another big change.
=
I am not. :-)

Sure, I'll send updated patch.
=C2=A0

--

Thanks &a= mp; Regards,

Ashesh Vashi
EnterpriseDB INDIA:=C2= =A0Enterp= rise PostgreSQL Company



Thanks.

On Fri, Jun 9, 2017 at 10:08 AM, Harshal Dhumal
<harshal.dhumal@enterprisedb.com> wrote:
> Hi,
>
>
> Please find rebased patch
>
> --
> Harshal Dhumal
> Sr. Software Engineer
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Thu, Jun 8, 2017 at 6:40 PM, Harshal Dhumal
> <harshal.dhumal@enterprisedb.com> wrote:
>>
>> Ignore this patch.
>> Rebase and migration of feature tests and jasmine tests required.<= br> >>
>> --
>> Harshal Dhumal
>> Sr. Software Engineer
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Thu, Jun 8, 2017 at 3:56 PM, Harshal Dhumal
>> <harshal.dhumal@enterprisedb.com> wrote:
>>>
>>> Hi,
>>> Please find attached updated patch for feature RM2137.
>>>
>>> Changes in this patch:
>>> 1. Patch rebased.
>>>
>>> 2. Updated existing feature tests which requires changes due t= o this
>>> feature.
>>>=C2=A0 =C2=A0 =C2=A0 affected feature test cases:
>>>=C2=A0 =C2=A0 =C2=A0 i. PGDataypeFeatureTest
>>>=C2=A0 =C2=A0 =C2=A0 ii. CheckForXssFeatureTest
>>>
>>> 3. Updated existing jasmine test cases which requires changes = due to this
>>> feature.
>>>=C2=A0 =C2=A0 =C2=A0 affected jasmine test cases:
>>>=C2=A0 =C2=A0 =C2=A0 i. copy data
>>>=C2=A0 =C2=A0 =C2=A0 ii. range_boundary_navigator
>>>=C2=A0 =C2=A0 =C2=A0 iii. row_selector
>>>=C2=A0 =C2=A0 =C2=A0 iv. set_stages_rows
>>>
>>> 4. New feature tests added
>>>=C2=A0 =C2=A0 =C2=A0i. on demand result set on scrolling.
>>>=C2=A0 =C2=A0 =C2=A0ii. on demand result set on grid select all= .
>>>=C2=A0 =C2=A0 =C2=A0iii. on demand result set on column select = all.
>>>=C2=A0 =C2=A0 =C2=A0iv. explain query
>>>=C2=A0 =C2=A0 =C2=A0v. explain query with verbose
>>>=C2=A0 =C2=A0 =C2=A0vi. explain query with costs
>>>=C2=A0 =C2=A0 =C2=A0vii. explain analyze query
>>>=C2=A0 =C2=A0 =C2=A0viii. explain analyze query with buffers >>>=C2=A0 =C2=A0 =C2=A0ix. explain analyze query with timing
>>>=C2=A0 =C2=A0 =C2=A0x. auto commit disabled.
>>>=C2=A0 =C2=A0 =C2=A0xi. auto commit enabled.
>>>=C2=A0 =C2=A0 =C2=A0xii. auto rollback enabled.
>>>=C2=A0 =C2=A0 =C2=A0xiii. cancel query.
>>>
>>>
>>>
>>> --
>>> Harshal Dhumal
>>> Sr. Software Engineer
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Tue, May 16, 2017 at 8:14 PM, Dave Page <dpage@pgadmin.org> wrote: >>>>
>>>>
>>>>
>>>> On Mon, May 15, 2017 at 7:40 PM, Harshal Dhumal
>>>> <harshal.dhumal@enterprisedb.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Sat, May 13, 2017 at 12:35 AM, Joao Pedro De Almeid= a Pereira
>>>>> <jdealmeidapereira@pivotal.io> wrote:
>>>>>>
>>>>>> We were only able to apply the patch on 1f903ba2 (= were seeing patch
>>>>>> does not apply due to sqleditor.js conflicts)
>>>>>> The javascript tests passed, but we were unable to= copy rows or
>>>>>> columns or cells when running the application. Cou= ld you run feature tests?
>>>>>
>>>>> There are three modes sqleditor can be launched
>>>>> 1. Query tool=C2=A0 (Tools menus -> Query Tool)
>>>>> 2. Datagrid.=C2=A0 (Right click on any table/view=C2= =A0 -> View Data -> View
>>>>> All/First 100/Last 100/Filtered rows)
>>>>> 3. Scripts (Right click on any table/view ->
>>>>> INSERT/CREATE/UPDATE/DELETE/SELECT)
>>>>>
>>>>> Paste functionality is only enabled in Datagrid and ta= ble has Primary
>>>>> key otherwise it's disabled. In your case row migh= t have been copied but you
>>>>> were unable paste because you might be trying to paste= the rows in Query
>>>>> tool. Please try again in Datagrid with table having P= rimary key.
>>>>>
>>>>>>
>>>>>>
>>>>>> Now that more functionality is being added to sqle= ditor.js, this may
>>>>>> be a good time to extract the functionality to sep= arate files. This will
>>>>>> increase readability, and encourage separation of = concerns. It will also
>>>>>> make changes easier to test in isolation.
>>>>>
>>>>> Ok. Let me check if I can separate out ant functionali= ties.
>>>>>
>>>>>>
>>>>>> It's probably a good idea to test the changes = made to the python as
>>>>>> well as javascript code. In this case, the new beh= avior of poll() in
>>>>>> sqleditor __init__ should be tested.
>>>>>
>>>>>
>>>>> At this point we don't have any python unit tests = that can test
>>>>> sqleditor backend (python code).
>>>>> @Dave should I include python unit test cases in this = patch?
>>>>
>>>>
>>>> We do have some feature tests that hit the query tool - Mu= rtuza wrote
>>>> some anti-XSS validation tests for example, and Khushboo h= as been working on
>>>> some datatype rendering tests.
>>>>
>>>> As a general rule, I prefer we focus more on feature tests= now than the
>>>> API tests - they cover the whole app end-to-end of course.= The disadvantages
>>>> are:
>>>>
>>>> - The treeview isn't reliable enough for me to enable = those tests on the
>>>> CI server yet.
>>>>
>>>> - They can take a long time to run, so we need to test mul= tiple things
>>>> at once wherever possible. That means minimising browser r= eloads, or new
>>>> instances of tools like the Query Tool - or even the numbe= r of queries
>>>> executed through the query tool as part of a test.
>>>>
>>>> That said, yes, if there are specific things that are not = covered by
>>>> Murtuza and Khushboo's work, we should test them. For = example, loading all
>>>> rows when the user selects all, running/rendering EXPLAIN,= auto-commit vs.
>>>> auto-rollback (and combinations thereof).
>>>>
>>>> The standard moving forwards should be to include feature = tests for new
>>>> functionality and Jasmine tests for algorithmic JS code. >>>>
>>>> I also agree with Joao on the modularisation of JS code. T= estable and
>>>> reusable code should be in "library" files, and = we should work to minimise
>>>> the amount of JS templates - for the most part, that means= moving to the
>>>> client-side translation mechanism which Tira worked on, an= d I've done some
>>>> early migration work.
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>
>>>
>>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-ha= ckers


--f403045f523a93636b0551ec54b5--