[sql] Complain about statement mutations after stepping has started.

sql/statement.cc

Index: sql/statement.cc
diff --git a/sql/statement.cc b/sql/statement.cc
index 67b7334c05709ea8ab51eb48429171910a9b9523..1f38e7691ce9601e6021a16d22d722037adc1639 100644
--- a/sql/statement.cc
+++ b/sql/statement.cc
@@ -16,11 +16,13 @@ namespace sql {
 // only have to check the ref's validity bit.
 Statement::Statement()
     : ref_(new Connection::StatementRef(NULL, NULL, false)),
+      stepped_(false),
       succeeded_(false) {
 }
 
 Statement::Statement(scoped_refptr<Connection::StatementRef> ref)
     : ref_(ref),
+      stepped_(false),
       succeeded_(false) {
 }
 
@@ -50,10 +52,12 @@ bool Statement::CheckValid() const {
 }
 
 bool Statement::Run() {
+  DCHECK(!stepped_);
   ref_->AssertIOAllowed();
   if (!CheckValid())
     return false;
 
+  stepped_ = true;
   return CheckError(sqlite3_step(ref_->stmt())) == SQLITE_DONE;
 }
 
@@ -62,6 +66,7 @@ bool Statement::Step() {
   if (!CheckValid())
     return false;
 
+  stepped_ = true;
   return CheckError(sqlite3_step(ref_->stmt())) == SQLITE_ROW;
 }
 
@@ -77,6 +82,7 @@ void Statement::Reset(bool clear_bound_vars) {
   }
 
   succeeded_ = false;
+  stepped_ = false;
 }
 
 bool Statement::Succeeded() const {

[Scott Hess - ex-Googler, 2013/10/24 19:51:37]

Almost tempted to put:
  DCHECK(stepped_);
here.

[Greg Billock, 2013/10/24 21:49:50]

Hmmmm. I hear you. Maybe a follow-up try run with that set would shake loose a
bug or two...

[Scott Hess - ex-Googler, 2013/10/25 17:50:36]

Fails SQLStatementTest.Run :-).

Thinking more about it, I'm no longer convinced about this notion.  Succeeded()
is documented as working this way.  I could kind of see a case where your
control-flow is such that it has a few cases, some of which optionally fire a
Step(), and then at the end it asks whether the statement has made any progress.

[I think that the is_valid() test may not be necessary, here.  succeeded_ can
never get to true if the statement isn't valid.]

@@ -87,6 +93,7 @@ bool Statement::Succeeded() const {
 }
 
 bool Statement::BindNull(int col) {
+  DCHECK(!stepped_);
   if (!is_valid())
     return false;
 
@@ -94,6 +101,7 @@ bool Statement::BindNull(int col) {
 }
 
 bool Statement::BindBool(int col, bool val) {
+  DCHECK(!stepped_);
   return BindInt(col, val ? 1 : 0);
 }
 
@@ -105,6 +113,7 @@ bool Statement::BindInt(int col, int val) {
 }
 
 bool Statement::BindInt64(int col, int64 val) {
+  DCHECK(!stepped_);
   if (!is_valid())
     return false;
 
@@ -112,6 +121,7 @@ bool Statement::BindInt64(int col, int64 val) {
 }
 
 bool Statement::BindDouble(int col, double val) {
+  DCHECK(!stepped_);
   if (!is_valid())
     return false;
 
@@ -119,6 +129,7 @@ bool Statement::BindDouble(int col, double val) {
 }
 
 bool Statement::BindCString(int col, const char* val) {
+  DCHECK(!stepped_);
   if (!is_valid())
     return false;
 
@@ -127,6 +138,7 @@ bool Statement::BindCString(int col, const char* val) {
 }
 
 bool Statement::BindString(int col, const std::string& val) {
+  DCHECK(!stepped_);
   if (!is_valid())
     return false;
 
@@ -142,6 +154,7 @@ bool Statement::BindString16(int col, const string16& value) {
 }
 
 bool Statement::BindBlob(int col, const void* val, int val_len) {
+  DCHECK(!stepped_);
   if (!is_valid())
     return false;