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

Hi Dave,

Please find rebased patch for RM2137.

--
*Harshal Dhumal*
*Sr. Software Engineer*

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

On Wed, Jun 14, 2017 at 7:55 PM, Harshal Dhumal <harshaldhumal15(at)gmail(dot)com>
wrote:

>
>
> On Wed, Jun 14, 2017 at 6:21 PM, Ashesh Vashi <
> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> On Wed, Jun 14, 2017 at 6:19 PM, Dave Page <dpage(at)pgadmin(dot)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.
>
>
>> --
>>
>> Thanks & Regards,
>>
>> Ashesh Vashi
>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> <http://www.enterprisedb.com/>
>>
>>
>> *http://www.linkedin.com/in/asheshvashi*
>> <http://www.linkedin.com/in/asheshvashi>
>>
>>>
>>> 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
>>>
>>>
>>> --
>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>
>>
>