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

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