Re: Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4]

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4]
Date: 2017-06-14 12:49:02
Message-ID: CA+OCxoxyAD1x_bEhzPS0RGU1SuL8Lfhx39ME7wY=L28ep4PdvQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.

Thanks.

On Fri, Jun 9, 2017 at 10:08 AM, Harshal Dhumal
<harshal(dot)dhumal(at)enterprisedb(dot)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(dot)dhumal(at)enterprisedb(dot)com> 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
>> <harshal(dot)dhumal(at)enterprisedb(dot)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 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 <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>
>>>>
>>>> On Mon, May 15, 2017 at 7:40 PM, Harshal Dhumal
>>>> <harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Sat, May 13, 2017 at 12:35 AM, Joao Pedro De Almeida Pereira
>>>>> <jdealmeidapereira(at)pivotal(dot)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. 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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2017-06-14 12:51:59 Re: Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4]
Previous Message Dave Page 2017-06-14 12:09:15 Re: Declarative partitioning in pgAdmin4