Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB-Manager: strip comments from SQL #9180

Closed

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented Feb 15, 2019

Fixes #21271

@elpaso elpaso added this to the 3.6.0 milestone Feb 15, 2019
@elpaso elpaso added the Bugfix label Feb 15, 2019
@@ -595,8 +595,14 @@ def createView(self):
def _getSqlQuery(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function is used to store queries in preset or in file. They are many usages. So we will loose comments in saved file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gustry ok, I'll move it down into the executor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gustry, looking better I thing you are wrong, the original behavior was altering the query by replacing newlines with spaces, and that was the source of the issue: https://github.com/qgis/QGIS/pull/9180/files#diff-9020444f364248acb9ad107f6fb38ba0L598

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSqlQuery is used when we store the query as file:

query = self._getSqlQuery()
so it means that the content of the file is not the same? (especially with this PR)

Sorry if I didn't understand. I will look later.

@elpaso
Copy link
Contributor Author

elpaso commented Feb 16, 2019

@Gustry can I merge?

@Gustry
Copy link
Contributor

Gustry commented Feb 16, 2019

As I said, the _getSqlQuery is called also when you save the query as a file or as a preset, so your PR removes comments from saved query.

In DB Manager:

-- this is a comment
SELECT * FROM foo;

Save this query as a preset or as a file, it becomes:

SELECT * FROM foo;

IMHO, we need another function like getExecutableSqlQuery which call first getSqlQuery and then clean the query (remove comments, replace \n, strip).

BTW, I noticed another one regression from QGIS 2, comments on the same line are not working.

SELECT *  --all fields
FROM foo;

is not working in QGIS 3.4 while it's working fine in QGIS 2.18 and PgAdmin3 and 4.

@elpaso
Copy link
Contributor Author

elpaso commented Feb 16, 2019

Superceeded by: #9183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants