Re: [pgAdmin4][Pattch] - RM #3673 - "Download as .csv" F8 does NOT work when one of joined files is a TEMPORARY file

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Pattch] - RM #3673 - "Download as .csv" F8 does NOT work when one of joined files is a TEMPORARY file
Date: 2019-02-20 12:16:18
Message-ID: CAFOhELcyykmfdhjGVVt1yz70XXGVQmR+ye+5Scq6jY0+_q=csg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

On Wed, Feb 20, 2019 at 5:22 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Thanks, patch applied!
>
> I did spot one other oddity in testing; we need to disable the search
> button/menu whilst queries are running (both to the grid and CSV). Can you
> whip up a quick fix for that please?
>
> The fix attached.

> Thanks!
>
> Thanks,
Khushboo

> On Wed, Feb 20, 2019 at 10:06 AM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Please find the attached updated patch.
>>
>> On Wed, Feb 20, 2019 at 10:56 AM Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Tue, Feb 19, 2019 at 7:27 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> I did some testing with this and found a couple of issues:
>>>>
>>>> - The toolbar isn't set correctly when the query is running. I can
>>>> still use buttons to modify the query for example (like indent selection),
>>>> and the cancel button is not enabled.
>>>>
>>> I have fixed it on my local machine.
>>>
>> Fixed.
>>
>>>
>>>> - I ran a very large query which generated millions of rows. It took a
>>>> few minutes to run before rendering the grid, but I killed it after 20
>>>> minutes or so when generating CSV. Is that because it's getting the entire
>>>> resultset to generate the CSV before downloading it, but with normal
>>>> output, it completes when it has enough data to display one batch of rows?
>>>>
>>>> Yes, right. We poll the data for rendering the grid in batches while in
>>> CSV we dump the entire resultset.
>>>
>>>> Also, the result grid seems to only be rendering itself to use about
>>>> 50% of the available vertical height now. Is that something messed up on my
>>>> system, or are you seeing it as well?
>>>>
>>>> This issue is the side effect of the Scratch Pad commit and has already
>>> been logged by Akshay. I will fix along with this RM.
>>>
>> Fixed.
>>
>>> Thanks.
>>>>
>>>> Thanks,
>>> Khushboo
>>>
>>>>
>>>> On Tue, Feb 19, 2019 at 5:39 AM Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Dave,
>>>>>
>>>>> On Mon, Feb 18, 2019 at 4:49 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> On Mon, Feb 18, 2019 at 10:51 AM Khushboo Vashi
>>>>>> <khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > On Mon, Feb 18, 2019 at 3:08 PM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>> wrote:
>>>>>> >>
>>>>>> >> On Mon, Feb 18, 2019 at 9:08 AM Khushboo Vashi
>>>>>> >> <khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>> >> >
>>>>>> >> >
>>>>>> >> >
>>>>>> >> > On Thu, Feb 14, 2019 at 4:12 PM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>> wrote:
>>>>>> >> >>
>>>>>> >> >> Hi
>>>>>> >> >>
>>>>>> >> >> On Thu, Feb 14, 2019 at 6:56 AM Khushboo Vashi
>>>>>> >> >> <khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>> >> >> >
>>>>>> >> >> > Hi,
>>>>>> >> >> >
>>>>>> >> >> > Please find the attached patch to fix the RM #3673 -
>>>>>> "Download as .csv" F8 does NOT work when one of joined files is a TEMPORARY
>>>>>> file
>>>>>> >> >> >
>>>>>> >> >> > To fix this issue, used the existing query tool connection
>>>>>> instead of a new connection to download the CSV file.
>>>>>> >> >>
>>>>>> >> >> That side of it seems to work well, however, I can still
>>>>>> attempt to
>>>>>> >> >> execute queries in the tool whilst it's running. We need to
>>>>>> display
>>>>>> >> >> the same gray screen with the spinner whilst a CSV download is
>>>>>> >> >> executing as we do when a normal query is executing. We also
>>>>>> need to
>>>>>> >> >> ensure the button bar behaves appropriately - e.g. the execute
>>>>>> options
>>>>>> >> >> should be disabled, the cancel button should be enabled etc.
>>>>>> >> >>
>>>>>> >> > We use an iframe to download the CSV file. So, after attaching
>>>>>> the proper URL to the iframe, the browser handles the download part.
>>>>>> >> > So, the main problem is catching the event after the download
>>>>>> completes.
>>>>>> >> >
>>>>>> >> > Any suggestion?
>>>>>> >>
>>>>>> >> We only used the iframe because we wanted to make it run
>>>>>> >> asynchronously didn't we?
>>>>>> >
>>>>>> > Yes, that's right. By using iframe the query tool page remains as
>>>>>> it is and side by side we download the file.
>>>>>> > So, now I can think of only one solution, to set a cookie just to
>>>>>> verify that the report has been sent to the browser and the connection is
>>>>>> now free now.
>>>>>>
>>>>>> Are there no examples of people doing something similar on the
>>>>>> internet? Seems like it might be a common problem.
>>>>>>
>>>>>>
>>>>> I have found 2 ways which are widely used to download the file:
>>>>>
>>>>> 1. *Iframe*
>>>>> - With this option, we can download the file without disturbing the
>>>>> current page which we have implemented, but the drawback is that the
>>>>> download part will be handled by the browser itself, so we can not catch
>>>>> the event where we can disable the query tool buttons and put the loader.
>>>>> 2. *Anchor tag with Download attribute with AJAX*
>>>>> - With this option, we can achieve what we required now. I have
>>>>> attached the patch for the same. One drawback, it is not supported on *Safari
>>>>> 10* which is our supported platform. It is supported on *Safari 10.1
>>>>> and *above
>>>>>
>>>>> Or does it even matter once the download has begun?
>>>>>>
>>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>> <https://postgresvision.com/>
>>>>> <https://postgresvision.com/>
>>>>>
>>>>
>>>>
>>>> --
>>>> 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
>

Attachment Content-Type Size
button_disable_fix.patch application/octet-stream 776 bytes

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2019-02-20 14:51:41 pgAdmin 4 commit: Update Docker README to match reality.
Previous Message Dave Page 2019-02-20 11:52:33 Re: [pgAdmin4][Pattch] - RM #3673 - "Download as .csv" F8 does NOT work when one of joined files is a TEMPORARY file