Обсуждение: [pgAdmin4][Pattch] - RM #3673 - "Download as .csv" F8 does NOT workwhen one of joined files is a TEMPORARY file

Hi

On Thu, Feb 14, 2019 at 6:56 AM Khushboo Vashi
<khushboo.vashi@enterprisedb.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.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

On Mon, Feb 18, 2019 at 9:08 AM Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
>
>
>
> On Thu, Feb 14, 2019 at 4:12 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Thu, Feb 14, 2019 at 6:56 AM Khushboo Vashi
>> <khushboo.vashi@enterprisedb.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?

As that's no longer a concern....

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

On Mon, Feb 18, 2019 at 10:51 AM Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
>
>
>
> On Mon, Feb 18, 2019 at 3:08 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Mon, Feb 18, 2019 at 9:08 AM Khushboo Vashi
>> <khushboo.vashi@enterprisedb.com> wrote:
>> >
>> >
>> >
>> > On Thu, Feb 14, 2019 at 4:12 PM Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Thu, Feb 14, 2019 at 6:56 AM Khushboo Vashi
>> >> <khushboo.vashi@enterprisedb.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.

Or does it even matter once the download has begun?

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

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

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?

Thanks.

On Tue, Feb 19, 2019 at 5:39 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Mon, Feb 18, 2019 at 4:49 PM Dave Page <dpage@pgadmin.org> wrote:

On Mon, Feb 18, 2019 at 10:51 AM Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
>
>
>
> On Mon, Feb 18, 2019 at 3:08 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Mon, Feb 18, 2019 at 9:08 AM Khushboo Vashi
>> <khushboo.vashi@enterprisedb.com> wrote:
>> >
>> >
>> >
>> > On Thu, Feb 14, 2019 at 4:12 PM Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Thu, Feb 14, 2019 at 6:56 AM Khushboo Vashi
>> >> <khushboo.vashi@enterprisedb.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

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Hi Dave,

On Tue, Feb 19, 2019 at 7:27 PM Dave Page <dpage@pgadmin.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.

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

Thanks.

Thanks,

Khushboo 

On Tue, Feb 19, 2019 at 5:39 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Mon, Feb 18, 2019 at 4:49 PM Dave Page <dpage@pgadmin.org> wrote:

On Mon, Feb 18, 2019 at 10:51 AM Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
>
>
>
> On Mon, Feb 18, 2019 at 3:08 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Mon, Feb 18, 2019 at 9:08 AM Khushboo Vashi
>> <khushboo.vashi@enterprisedb.com> wrote:
>> >
>> >
>> >
>> > On Thu, Feb 14, 2019 at 4:12 PM Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Thu, Feb 14, 2019 at 6:56 AM Khushboo Vashi
>> >> <khushboo.vashi@enterprisedb.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

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Hi Dave,

On Wed, Feb 20, 2019 at 5:22 PM Dave Page <dpage@pgadmin.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.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached updated patch.

On Wed, Feb 20, 2019 at 10:56 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Tue, Feb 19, 2019 at 7:27 PM Dave Page <dpage@pgadmin.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.vashi@enterprisedb.com> wrote:

Hi Dave,

On Mon, Feb 18, 2019 at 4:49 PM Dave Page <dpage@pgadmin.org> wrote:

On Mon, Feb 18, 2019 at 10:51 AM Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
>
>
>
> On Mon, Feb 18, 2019 at 3:08 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Mon, Feb 18, 2019 at 9:08 AM Khushboo Vashi
>> <khushboo.vashi@enterprisedb.com> wrote:
>> >
>> >
>> >
>> > On Thu, Feb 14, 2019 at 4:12 PM Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Thu, Feb 14, 2019 at 6:56 AM Khushboo Vashi
>> >> <khushboo.vashi@enterprisedb.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

--

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

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. 

Hi

Thanks - that fixed the double scrollbar, but not the other issue. Any ideas on that one?

On Wed, Feb 27, 2019 at 5:52 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

Please find the attached fix.

I have added the configurable option to add the extra classes in the panel.js, so in the future, we can utilize it if required.

Here, I have added a class to hide the extra vertical scrollbar to the Datagrid panel.

Thanks,

Khushboo

On Tue, Feb 26, 2019 at 7:08 PM Dave Page <dpage@pgadmin.org> wrote:

Hi Khushboo,

On Wed, Feb 20, 2019 at 10:06 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

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. 

I've started seeing weirdness with the Query Tool grid again. If I run View Data on a table with more rows than can be seen on screen at once, I get a double scrollbar. If I then resize the grid, the double scrollbar vanishes, but the grid re-renders incorrectly. Screenshots attached.

Can you take a look please?

--

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

Hi Dave,

On Wed, Feb 27, 2019 at 4:30 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

Thanks - that fixed the double scrollbar, but not the other issue. Any ideas on that one?

Can you please provide the exact steps to reproduce the second issue as after fixing the first one I can't reproduce it. 

Hi

On Thu, Feb 28, 2019 at 3:46 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Wed, Feb 27, 2019 at 4:30 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

Thanks - that fixed the double scrollbar, but not the other issue. Any ideas on that one?

Can you please provide the exact steps to reproduce the second issue as after fixing the first one I can't reproduce it. 

Sure - movie seems easiest, so grab some popcorn and put the attached on the big screen! 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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

Hi Dave,

Please find the attached fix.

Thanks,

Khushboo

On Thu, Feb 28, 2019 at 6:58 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, Feb 28, 2019 at 3:46 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Wed, Feb 27, 2019 at 4:30 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

Thanks - that fixed the double scrollbar, but not the other issue. Any ideas on that one?

Can you please provide the exact steps to reproduce the second issue as after fixing the first one I can't reproduce it. 

Sure - movie seems easiest, so grab some popcorn and put the attached on the big screen! 

Unfortunately, I don't have a big screen, so I missed the opportunity to have popcorn. :)

On Fri, Mar 1, 2019 at 5:39 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

Please find the attached fix.

Thanks,

Khushboo

On Thu, Feb 28, 2019 at 6:58 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, Feb 28, 2019 at 3:46 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Wed, Feb 27, 2019 at 4:30 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

Thanks - that fixed the double scrollbar, but not the other issue. Any ideas on that one?

Can you please provide the exact steps to reproduce the second issue as after fixing the first one I can't reproduce it. 

Sure - movie seems easiest, so grab some popcorn and put the attached on the big screen! 

Unfortunately, I don't have a big screen, so I missed the opportunity to have popcorn. :)

Booo :-(

Well, anyway - the sizing issue seems OK now, but if I have a table (id serial primary key, data text) with enough rows that I need to scroll, when I scroll to the end I can edit the id field in place, but the text editor popup doesn't show up. If I try to edit a row further up, it seems to appear in a weird place, so I suspect it's just rendering off-screen.

I think we should render it directly against the cell - above if the cell is >50% down in the overall screen, else above, and something similarly sane for the left/right.

Not sure if this is a regression or not, but it's unusable as-is, so can you fix that too? I'll commit your current patch.

Thanks!

 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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