Skip to content

Conversation

@jwarlander
Copy link

While integrating the query wrapper into one of our loading scripts, I noticed as I was testing the outcome that even if a wrapper:query(...) call failed, and aborted the script, previous database updates within the same script would still be committed.

This happens because wrap_query, in case of an error and where self.on_error == 'abort', calls self:finish(), which in the default implementation proceeds to just write the log entries and do a self:commit().

As this was somewhat surprising to me, I felt that perhaps wrap_finish should do a rollback before writing the log entries if there were any errors.

If one doesn't want a rollback to happen in case of errors, it's easy enough to set self.on_error to something else than abort, and handle errors manually.

Alternatively, one can also do some checkpointing by just calling wrapper:commit() at well-chosen places throughout a script, if there are partial changes that should be preserved.

@jwarlander
Copy link
Author

Any comments, or obstacles for this change getting merged? :) If rollback on error isn't the wanted behavior, then that's fine, of course; just let me know!

@jwarlander
Copy link
Author

@exasol-szba
Copy link
Contributor

Ping.. @ThomasBestfleisch, @exasol-szba, @exaValerieWiedemann :)

Hallo, hallo!
Sorry for not noticing your communication earlier; this was the first time GitHub notified me in relation to this thread.
I will look into this. I think that the whole logging part should be rewritten as there are other problems with it (although they are more cosmetic).

@exasol-szba exasol-szba self-assigned this Jan 24, 2019
@jwarlander
Copy link
Author

Hello!
No worries.. let me know if there's anything I can help out with :)

@jschildgen
Copy link
Contributor

We added an option on_error = 'rollback' (same as abort, but does not commit but rollback in case of an error). This should fix your problem.

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.

3 participants