[issue10740] sqlite3 module breaks transactions and potentially corrupts data

[R. David Murray]

R. David Murray added the comment:

On Thu, 09 Jan 2014 20:29:15 +0000, Adam Tomjack <report at bugs.python.org> wrote:
> This issue is not an enhancement, it's a bug.  One might argue that
> fixing it will break existing code, but it won't.  Any affected code
> is already broken.  It's depending on a promised level of safety, and
> this bug is breaking that promise.  It's better to fail noisily and
> consistently than to usually work, but sometimes silently corrupt
> data.

That's not how our backward compatibility policy works.  If a program
runs correctly, a change in a maintenance release should not cause
that program to fail, *even if the change is a bug fix*.

Now, that said, silent data corruption can change that calculus, but
in this case there are many many working programs out there that
don't experience data corruption, so breaking them in a maintenance
release is just not acceptable.

It is possible that there are specific things that can be fixed without
breaking backward compatibility, but they will have to be argued individually.

> In general, it's a code-smell to inspect the SQL that's being executed
> before sending it to the database.  The only reason I can think of to

There are reasons why the sqlite module was designed the way it was,
having to do with an intersection between how sqlite works and the DB
API 2 specification.  I'm not saying the design (and implementation of
that design) has no bugs, but there *are* reasons why the choices made
were made, including the concurrency issue mentioned by Mike (which if I
understand correctly was even more problematic in earlier versions of
sqlite.)

> inspect SQL is to work around brokenness in the underlying database.
> Suppose, for example, that SQLite implicitly committed when it got
> DDL.  (I don't know what it actually does.  Just suppose.)  In that

By default, SQLite starts a transaction when any command that changes
the database is executed (the docs say, "basically, anything other than
a SELECT), and "automatically started transactions are committed when
the last query finishes".  I'm not sure exactly what that means.
Note that there is no way in sqlite to turn auto-transaction behavior 
off.

> case, it would be necessary to check for DDL and raise an exception
> before passing the statement to the database.  If the database

Like I said, this would need to be a new feature, due to our backward
compatibility policy.

> correctly errors in such a situation, then the python wrapper should
> just pass the DDL to the database and deal with the resulting error.
> That's way easier that trying to correctly detect what the statement
> type is.  Parsing SQL correctly is hard.

If you want full control of transactions while using the sqlite module,
you can set the isolation_level to None (sqlite's default autocommit
mode) and then carefully control when you issue what kinds of statements,
including BEGIN statements.  Any exceptions produced by the database
will in that situation be propagated to the application.  That loses
the (admittedly somewhat limited) DB-interoperability benefits of using
the DB API 2, though.

> As evidence of that, note that the existing statement detection code
> is broken: it doesn't strip comments first!  A simple SELECT statement
> preceeded by a comment will implicitly commit!  But commenting is
> good.

Yes, the fact that statement detection (and what statements are
detected) is buggy is the essence of this issue.

But note that in Python code it would be much more natural to
put the comment in the python code, not the sql string.

> Anything that causes the underlying database to implicitly commit must
> be avoided (yet the python module goes out of it's way to add *MORE*
> such buggy behavior!)  Fixing brokenness in the underlying database is
> the only reason to inspect SQL as it passes through this module.  If

Yep.

> there are other ways to avoid such problems, they will likely be
> better than inspecting SQL.

Do you have a better way to suggest?

> Anything which causes implicit rollbacks in the underlying database is
> a similar issue, but easier to handle without parsing SQL.  It's not

There are no implicit rollbacks at issue anywhere here, that I can see.
Do you have an example where the module does an implicit rollback?

> Here is an example demonstrating that a SELECT can trigger an implicit
> commit.

Yes, the comment issue should be fixed.  That's a different bug, though, 
and should be addressed in a separate issue.  Unfortunately, since
some people seem to be *depending* on the fact that putting in a comment
disables statement detection, that, too will need to be a new feature.

I'm sorry about that, but Python has a strong commitment to backward
compatibility.

----------
type: behavior -> enhancement
versions: +Python 3.5 -Python 2.7, Python 3.1, Python 3.2, Python 3.3

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue10740>
_______________________________________