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

Refactor the query execution code in Execute Query tab #1181

Merged
merged 3 commits into from Oct 27, 2017

Conversation

Projects
None yet
2 participants
@MKleusberg
Copy link
Member

MKleusberg commented Oct 20, 2017

This is an attempt to start refactoring one of my most dreaded bits of code in DB4S. It's still not awesome but it's definitely starting to become easier to understand.

As with the last PRs, I'll leave this here for a couple of days for feedback and will merge this afterwards.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 20, 2017

If it helps, I can take a look at this on Monday?

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Oct 20, 2017

That would be awesome 😄 I'll try to add an extra commit over the weekend to this. So really no need to start testing before Monday. And if that extra commit doesn't work out as expected, I will let you know.

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Oct 20, 2017

Hmm, nevermind for now. This totally breaks CTE queries and it doesn't seem easy to fix it.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 21, 2017

No worries. 😄

@MKleusberg MKleusberg force-pushed the refactored_query_exec branch from 794270e to a535c37 Oct 22, 2017

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg commented Oct 22, 2017

Ok, I've reverted most of the changes and changed this PR to only include less drastic changes 😄

Before this we made the assumption that statements starting with SELECT are read-only and that all other statements would write to the database. This isn't true for reading PRAGMA statements and for recursive statements (starting with WITH). After the proposed changes the assumption is that statements which return data (including empty result sets) are read-only and that statements which don't return data might make changes to the database. I'm not 100% sure that this is always the case, so I'll leave this some more days open for discussion.

And please ignore the first three commits which were added today. They are only listed here because I'm stupid and because Github is just as stupid, too.

@MKleusberg MKleusberg referenced this pull request Oct 22, 2017

Closed

Executing select "with recursion" marks database dirty #1185

5 of 14 tasks complete

MKleusberg added some commits Oct 22, 2017

Remove the valid flag from the SqliteTableModel class
Remove the valid flag from the SqliteTableModel class and remove its
usage in the Execute SQL tab of the main window. I believe this hasn't
been used for some time now because the main sources of error should
really be noticed before the query is even handed over to the model.
Additionally the valid flag wasn't as realible either anymore because of
the multi-threaded execution of the model queries.
Improve detection of query vs. modify statements in Execute SQL tab
This should improve the detection of read-only query statements vs.
modifiying statements in the Execute SQL tab. The idea is to stop
looking for the SELECT keyword at the beginning of the statement and
instead fully rely on whether SQLite returns any data for this
statement.

See issue #1185.

@MKleusberg MKleusberg force-pushed the refactored_query_exec branch from c385e32 to 2ef07e5 Oct 27, 2017

@MKleusberg MKleusberg merged commit 18b7814 into master Oct 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MKleusberg MKleusberg deleted the refactored_query_exec branch Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment