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

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, 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:51:59
Message-ID: CAG7mmoxASRd2wTahma6WNLCLfg=4gy5eOQ+e5K0ioOHPUS+7bA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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. :-)

--

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
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-06-14 13:11:56 pgAdmin 4 commit: Add some more Makefile targets for running different
Previous Message Dave Page 2017-06-14 12:49:02 Re: Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4]