public inbox for [email protected]help / color / mirror / Atom feed
[pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL 7+ messages / 2 participants [nested] [flat]
* [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL @ 2016-09-19 14:03 Surinder Kumar <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Surinder Kumar @ 2016-09-19 14:03 UTC (permalink / raw) To: pgadmin-hackers Hi Please find attached patch with fix. > Download as CSV > button should be enabled only for SELECT queries. Please review. Thanks, Surinder Kumar -- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers Attachments: [application/octet-stream] RM1592.patch (1.6K, 3-RM1592.patch) download | inline diff: diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js index 74502c7..8f33459 100644 --- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js @@ -1593,10 +1593,17 @@ define( $("#btn-file-menu-dropdown").prop('disabled', true); $("#btn-copy-row").prop('disabled', true); $("#btn-paste-row").prop('disabled', true); + $("#btn-download").prop('disabled', false); // Set the combo box value $(".limit").val(res.data.limit); + var editor_query = self.query; + if (editor_query && _.isNull(editor_query.toLowerCase(). + match('^select'))) { + $("#btn-download").prop('disabled', true); + } + // If status is True then poll the result. self._poll(); } @@ -2963,6 +2970,15 @@ define( self.disable_tool_buttons(true); $("#btn-cancel-query").prop('disabled', false); + var editor_query = sql; + if (editor_query && _.isNull(editor_query.toLowerCase(). + match('^select'))) { + $("#btn-download").prop('disabled', true); + } + else { + $("#btn-download").prop('disabled', false); + } + $.ajax({ url: "{{ url_for('sqleditor.index') }}" + "query_tool/start/" + self.transId, method: 'POST', ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL @ 2016-09-19 15:18 Dave Page <[email protected]> parent: Surinder Kumar <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Dave Page @ 2016-09-19 15:18 UTC (permalink / raw) To: Surinder Kumar <[email protected]>; +Cc: pgadmin-hackers Hi On Mon, Sep 19, 2016 at 3:03 PM, Surinder Kumar <[email protected]> wrote: > Hi > > Please find attached patch with fix. >> >> Download as CSV >> button should be enabled only for SELECT queries. I'm not sure there's a way to do this without parsing the query. Simply matching on ^select certainly won't work reliably - for example, there could be multiple statements in the script, or it could be PERFORM ... or UPDATE ... RETURNING ... I think we need to leave the button enabled, but give the user a message if no data is returned, e.g. ======= Download as CSV ------------------------- The query executed did not return any data. [OK] ======== -- 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 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL @ 2016-09-19 15:29 Surinder Kumar <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Surinder Kumar @ 2016-09-19 15:29 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: pgadmin-hackers On Mon, Sep 19, 2016 at 8:48 PM, Dave Page <[email protected]> wrote: > Hi > > On Mon, Sep 19, 2016 at 3:03 PM, Surinder Kumar > <[email protected]> wrote: > > Hi > > > > Please find attached patch with fix. > >> > >> Download as CSV > >> button should be enabled only for SELECT queries. > > I'm not sure there's a way to do this without parsing the query. > Simply matching on ^select certainly won't work reliably - for > example, there could be multiple statements in the script, or it could > be PERFORM ... or UPDATE ... RETURNING ... > I wasn't aware of such cases. > > I think we need to leave the button enabled, but give the user a > message if no data is returned, e.g. > ok. sure. > > ======= > Download as CSV > ------------------------- > > The query executed did not return any data. > > [OK] > ======== > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL @ 2016-09-20 05:43 Surinder Kumar <[email protected]> parent: Surinder Kumar <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Surinder Kumar @ 2016-09-20 05:43 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: pgadmin-hackers On Mon, Sep 19, 2016 at 8:59 PM, Surinder Kumar < [email protected]> wrote: > On Mon, Sep 19, 2016 at 8:48 PM, Dave Page <[email protected]> wrote: > >> Hi >> >> On Mon, Sep 19, 2016 at 3:03 PM, Surinder Kumar >> <[email protected]> wrote: >> > Hi >> > >> > Please find attached patch with fix. >> >> >> >> Download as CSV >> >> button should be enabled only for SELECT queries. >> >> I'm not sure there's a way to do this without parsing the query. >> Simply matching on ^select certainly won't work reliably - for >> example, there could be multiple statements in the script, or it could >> be PERFORM ... or UPDATE ... RETURNING ... >> > I wasn't aware of such cases. > >> >> I think we need to leave the button enabled, but give the user a >> message if no data is returned, e.g. >> > ok. sure. > I discussed the issue with Harshal and this is how download csv works: We just hit the url to download csv, and let the browser handle the response from the server side. we don't have access to response data to check if no data is returned. The other approach is to hit the same query twice. First time an ajax query to check if data is returned or not. if returned, fire another query to download csv otherwise set message in message panel. But the limitation with this approach is that we are just increasing load over server by hitting same query twice, incase returned data rows in billions. Thoughts ? > >> ======= >> Download as CSV >> ------------------------- >> >> The query executed did not return any data. >> >> [OK] >> ======== > > >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL @ 2016-09-20 09:03 Dave Page <[email protected]> parent: Surinder Kumar <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Dave Page @ 2016-09-20 09:03 UTC (permalink / raw) To: Surinder Kumar <[email protected]>; +Cc: pgadmin-hackers On Tue, Sep 20, 2016 at 6:43 AM, Surinder Kumar <[email protected]> wrote: > > On Mon, Sep 19, 2016 at 8:59 PM, Surinder Kumar > <[email protected]> wrote: >> >> On Mon, Sep 19, 2016 at 8:48 PM, Dave Page <[email protected]> wrote: >>> >>> Hi >>> >>> On Mon, Sep 19, 2016 at 3:03 PM, Surinder Kumar >>> <[email protected]> wrote: >>> > Hi >>> > >>> > Please find attached patch with fix. >>> >> >>> >> Download as CSV >>> >> button should be enabled only for SELECT queries. >>> >>> I'm not sure there's a way to do this without parsing the query. >>> Simply matching on ^select certainly won't work reliably - for >>> example, there could be multiple statements in the script, or it could >>> be PERFORM ... or UPDATE ... RETURNING ... >> >> I wasn't aware of such cases. >>> >>> >>> I think we need to leave the button enabled, but give the user a >>> message if no data is returned, e.g. >> >> ok. sure. > > I discussed the issue with Harshal and this is how download csv works: > We just hit the url to download csv, and let the browser handle the response > from the server side. we don't have access to response data to check if no > data is returned. Hmm, yeah that's true. > The other approach is to hit the same query twice. First time an ajax query > to check if data is returned or not. if returned, fire another query to > download csv otherwise set message in message panel. > But the limitation with this approach is that we are just increasing load > over server by hitting same query twice, incase returned data rows in > billions. > Thoughts ? Yeah, we can't do that. The only thing I can think of right now is to simply write the error message to the CSV file, e.g. "The query executed did not return any data." (with the quotes, so it's valid CSV). That way at least the user will always get a file downloaded, and will see the error when they open it. -- 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 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL @ 2016-09-20 10:39 Surinder Kumar <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Surinder Kumar @ 2016-09-20 10:39 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: pgadmin-hackers Please find updated patch with suggested changes and inline comments. Also, Incase if user tries to download csv on queries other than select, we are catching that exception and write into 'csv' with name 'error.csv'. On Tue, Sep 20, 2016 at 2:33 PM, Dave Page <[email protected]> wrote: > On Tue, Sep 20, 2016 at 6:43 AM, Surinder Kumar > <[email protected]> wrote: > > > > On Mon, Sep 19, 2016 at 8:59 PM, Surinder Kumar > > <[email protected]> wrote: > >> > >> On Mon, Sep 19, 2016 at 8:48 PM, Dave Page <[email protected]> wrote: > >>> > >>> Hi > >>> > >>> On Mon, Sep 19, 2016 at 3:03 PM, Surinder Kumar > >>> <[email protected]> wrote: > >>> > Hi > >>> > > >>> > Please find attached patch with fix. > >>> >> > >>> >> Download as CSV > >>> >> button should be enabled only for SELECT queries. > >>> > >>> I'm not sure there's a way to do this without parsing the query. > >>> Simply matching on ^select certainly won't work reliably - for > >>> example, there could be multiple statements in the script, or it could > >>> be PERFORM ... or UPDATE ... RETURNING ... > >> > >> I wasn't aware of such cases. > >>> > >>> > >>> I think we need to leave the button enabled, but give the user a > >>> message if no data is returned, e.g. > >> > >> ok. sure. > > > > I discussed the issue with Harshal and this is how download csv works: > > We just hit the url to download csv, and let the browser handle the > response > > from the server side. we don't have access to response data to check if > no > > data is returned. > > Hmm, yeah that's true. > > > The other approach is to hit the same query twice. First time an ajax > query > > to check if data is returned or not. if returned, fire another query to > > download csv otherwise set message in message panel. > > But the limitation with this approach is that we are just increasing load > > over server by hitting same query twice, incase returned data rows in > > billions. > > Thoughts ? > > Yeah, we can't do that. > > The only thing I can think of right now is to simply write the error > message to the CSV file, e.g. > > "The query executed did not return any data." > > (with the quotes, so it's valid CSV). That way at least the user will > always get a file downloaded, and will see the error when they open > it. > Thats' done. > > -- > 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 Attachments: [application/octet-stream] RM1592_v1.patch (2.5K, 3-RM1592_v1.patch) download | inline diff: diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 2c96dea..8e78967 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -1304,16 +1304,18 @@ def start_query_download_tool(trans_id): sync_conn.connect(autocommit=False) + def cleanup(): + conn.manager.connections[sync_conn.conn_id]._release() + del conn.manager.connections[sync_conn.conn_id] + # This returns generator of records. status, gen = sync_conn.execute_on_server_as_csv(sql, records=2000) if not status: - conn.manager.release(conn_id=conn_id, did=trans_obj.did) - return internal_server_error(errormsg=str(gen)) - - def cleanup(): - conn.manager.connections[sync_conn.conn_id]._release() - del conn.manager.connections[sync_conn.conn_id] + r = Response('"{0}"'.format(gen), mimetype='text/csv') + r.headers["Content-Disposition"] = "attachment;filename=error.csv" + r.call_on_close(cleanup) + return r r = Response(gen(), mimetype='text/csv') @@ -1326,11 +1328,12 @@ def start_query_download_tool(trans_id): r.headers["Content-Disposition"] = "attachment;filename={0}".format(filename) r.call_on_close(cleanup) - return r except Exception as e: - conn.manager.release(conn_id=conn_id, did=trans_obj.did) - return internal_server_error(errormsg=str(e)) + r = Response('"{0}"'.format(e), mimetype='text/csv') + r.headers["Content-Disposition"] = "attachment;filename=error.csv" + r.call_on_close(cleanup) + return r else: return internal_server_error(errormsg=gettext("Transaction status check failed.")) diff --git a/web/pgadmin/utils/driver/psycopg2/__init__.py b/web/pgadmin/utils/driver/psycopg2/__init__.py index 1f7773b..2d6239f 100644 --- a/web/pgadmin/utils/driver/psycopg2/__init__.py +++ b/web/pgadmin/utils/driver/psycopg2/__init__.py @@ -609,6 +609,7 @@ WHERE if not results: if not cur.closed: cur.close() + yield gettext('"The query executed did not return any data."') return header = [c.to_dict()['name'] for c in cur.ordered_description()] ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL @ 2016-09-20 10:58 Dave Page <[email protected]> parent: Surinder Kumar <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Dave Page @ 2016-09-20 10:58 UTC (permalink / raw) To: Surinder Kumar <[email protected]>; +Cc: pgadmin-hackers Thanks, applied. On Tue, Sep 20, 2016 at 11:39 AM, Surinder Kumar <[email protected]> wrote: > Please find updated patch with suggested changes and inline comments. > > Also, Incase if user tries to download csv on queries other than select, we > are catching that exception and write into 'csv' with name 'error.csv'. > > On Tue, Sep 20, 2016 at 2:33 PM, Dave Page <[email protected]> wrote: >> >> On Tue, Sep 20, 2016 at 6:43 AM, Surinder Kumar >> <[email protected]> wrote: >> > >> > On Mon, Sep 19, 2016 at 8:59 PM, Surinder Kumar >> > <[email protected]> wrote: >> >> >> >> On Mon, Sep 19, 2016 at 8:48 PM, Dave Page <[email protected]> wrote: >> >>> >> >>> Hi >> >>> >> >>> On Mon, Sep 19, 2016 at 3:03 PM, Surinder Kumar >> >>> <[email protected]> wrote: >> >>> > Hi >> >>> > >> >>> > Please find attached patch with fix. >> >>> >> >> >>> >> Download as CSV >> >>> >> button should be enabled only for SELECT queries. >> >>> >> >>> I'm not sure there's a way to do this without parsing the query. >> >>> Simply matching on ^select certainly won't work reliably - for >> >>> example, there could be multiple statements in the script, or it could >> >>> be PERFORM ... or UPDATE ... RETURNING ... >> >> >> >> I wasn't aware of such cases. >> >>> >> >>> >> >>> I think we need to leave the button enabled, but give the user a >> >>> message if no data is returned, e.g. >> >> >> >> ok. sure. >> > >> > I discussed the issue with Harshal and this is how download csv works: >> > We just hit the url to download csv, and let the browser handle the >> > response >> > from the server side. we don't have access to response data to check if >> > no >> > data is returned. >> >> Hmm, yeah that's true. >> >> > The other approach is to hit the same query twice. First time an ajax >> > query >> > to check if data is returned or not. if returned, fire another query to >> > download csv otherwise set message in message panel. >> > But the limitation with this approach is that we are just increasing >> > load >> > over server by hitting same query twice, incase returned data rows in >> > billions. >> > Thoughts ? >> >> Yeah, we can't do that. >> >> The only thing I can think of right now is to simply write the error >> message to the CSV file, e.g. >> >> "The query executed did not return any data." >> >> (with the quotes, so it's valid CSV). That way at least the user will >> always get a file downloaded, and will see the error when they open >> it. > > Thats' done. >> >> >> -- >> 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 ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2016-09-20 10:58 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2016-09-19 14:03 [pgAdmin4][Patch]: RM1592 - Download as CSV should be supported for DDL Surinder Kumar <[email protected]> 2016-09-19 15:18 ` Dave Page <[email protected]> 2016-09-19 15:29 ` Surinder Kumar <[email protected]> 2016-09-20 05:43 ` Surinder Kumar <[email protected]> 2016-09-20 09:03 ` Dave Page <[email protected]> 2016-09-20 10:39 ` Surinder Kumar <[email protected]> 2016-09-20 10:58 ` Dave Page <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox