public inbox for [email protected]
help / color / mirror / Atom feedFrom: Harshal Dhumal <[email protected]>
To: Ashesh Vashi <[email protected]>
Cc: Dave Page <[email protected]>
Cc: Harshal Dhumal <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4]
Date: Wed, 14 Jun 2017 19:55:36 +0530
Message-ID: <CAE29nRn0Z7v4oOWWndLr1Cr8HRwHizBK3+7BK12KnT0uHHHi7Q@mail.gmail.com> (raw)
In-Reply-To: <CAG7mmoxASRd2wTahma6WNLCLfg=4gy5eOQ+e5K0ioOHPUS+7bA@mail.gmail.com>
References: <CAFiP3vxaM9ixpmaM04JAkTT1ONwmnMPwUCbK+kzCN+r6F+7-ZA@mail.gmail.com>
<CA+OCxozb7qDTiskHXv08G5TrCi7zmhR7Csz=x9V4ermehyO8cg@mail.gmail.com>
<CAFiP3vzas6ez9u-snfCuXzb1eL6_aZwAfrb+561sq8V19PT4Eg@mail.gmail.com>
<CA+OCxoy-8RyH-QYxXkLn1KSTUJMArev0ukKpfJx+yvATEyJV+Q@mail.gmail.com>
<CAFiP3vz6_Up4OfSAR4J8C35EzgdApbAiZCeZzzCKxnjwgJMdYQ@mail.gmail.com>
<CAGRPzo_KmDF=ThGC9hy6JepsxBn23nuXkamJBQgT6eOFh0p1pg@mail.gmail.com>
<CAFiP3vwYLTLXX5EFPzE_s-0ucQikDERp_iS+4Neb1JN-sHfG9A@mail.gmail.com>
<CAFiP3vxvFMHv=yEOXcbxi9YABtTwOk3Feci92YMA4_9Fcp-4mA@mail.gmail.com>
<CAFiP3vzj2HBjau+N5TN+Asb_7DW6zgmPrzJf0rmd_H0NJ3Sttg@mail.gmail.com>
<CAE+jjamhZ27XJ2ZHGRgpsL0yUUOawDhiMXoxfB-bRCu=M8RBhg@mail.gmail.com>
<CAFiP3vw=rGTY1BFPB2ZLjLBehEaPpXgXRY2SerK+07TrA+nxzA@mail.gmail.com>
<CA+OCxozQj0gy7dsorij5fXSgLuhGDJP7uC4++VviAPR1XFs-Fw@mail.gmail.com>
<CAFiP3vyjbmQN2bRpHJvNXFnK6YvtgPm12OiF_Oa2Z7J73=C3Ag@mail.gmail.com>
<CAFiP3vzomo+37TZ87tVr4AsmUNzzsFQkq8j8fEXNKdpVj9bOxw@mail.gmail.com>
<CAFiP3vz1EGT4cgv3XJNT6wLDz0Ca7dT_W0p6GsDVPua+tWZrpw@mail.gmail.com>
<CA+OCxoxyAD1x_bEhzPS0RGU1SuL8Lfhx39ME7wY=L28ep4PdvQ@mail.gmail.com>
<CAG7mmoxASRd2wTahma6WNLCLfg=4gy5eOQ+e5K0ioOHPUS+7bA@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
On Wed, Jun 14, 2017 at 6:21 PM, Ashesh Vashi <[email protected]
> wrote:
> On Wed, Jun 14, 2017 at 6:19 PM, Dave Page <[email protected]> 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
>> <[email protected]> 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
>> > <[email protected]> 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
>> >> <[email protected]> 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 <[email protected]> wrote:
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Mon, May 15, 2017 at 7:40 PM, Harshal Dhumal
>> >>>> <[email protected]> wrote:
>> >>>>>
>> >>>>> Hi,
>> >>>>>
>> >>>>> On Sat, May 13, 2017 at 12:35 AM, Joao Pedro De Almeida Pereira
>> >>>>> <[email protected]> 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 ([email protected])
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>
>
view thread (24+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4]
In-Reply-To: <CAE29nRn0Z7v4oOWWndLr1Cr8HRwHizBK3+7BK12KnT0uHHHi7Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox