Add try/catch to cursor.fetchall() #7504
Conversation
I use SQL_QUERY_MUTATOR to add some sql for changing role before the main query. Some of them doesn't return data, and causes exceptions here.
| try: | ||
| cursor.fetchall() | ||
| except BaseException: | ||
| pass |
There was a problem hiding this comment.
Hmm, I might be missing something obvious here, but why are we calling fetchall() if we're not even storing the return value anywhere? Wouldn't it be enough to just call exexute() here and remove the fetch all together?
There was a problem hiding this comment.
I was confused about here too. We only want data from the last query.
There was a problem hiding this comment.
Aah yes, it's explained here: #5517. I think this looks good, but it might be good to add a comment explaining that some engines require calling fetchall() to force the execute() calls to execute synchronously.
There was a problem hiding this comment.
@Danielking could you provide more context (as well as a stack trace) as to when this fails. Note in general SQL exceptions should not be swallowed but captured and presented in the explorer as a message to inform the user of the problem.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue |
I use SQL_QUERY_MUTATOR to add some sql for changing role before the main query. Some of them doesn't return data, and causes exceptions here.
CATEGORY
Choose one
SUMMARY
I use SQL_QUERY_MUTATOR to add some additional queries for changing role before the main query. Some of them doesn't return data, and causes exception here.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION
REVIEWERS