570529 - mozIStorageStatement.execute() should reset itself even if an error occurs

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Dave Townsend [:mossop]]

mozIStorageStatement.execute reads to me like after it is complete the statement should be ready for re-use. However in the event of an error occurring the statement does not get reset.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Kinda weird with the multiple nsresults but I think we want to return the result from ExecuteStep if it was a failure or the result from Reset otherwise.

Comment 2

[Shawn Wilsher :sdwilsh]

Comment on attachment 449677 [details] [diff] [review]
patch rev 1

>+function test_failed_execute()
>+{
>+  var stmt = createStatement("INSERT INTO test (name) VALUES ('foo')");
>+  stmt.execute();
>+  stmt.finalize();
>+  var id = getOpenedDatabase().lastInsertRowID;
>+  stmt = createStatement("INSERT INTO test(id, name) VALUES(:id, 'bar')");
>+  stmt.params.id = id;
>+  try {
>+    // Should throw a constraint error
>+    stmt.execute();
>+    do_throw("Should have seen a constraint error");
>+  }
>+  catch (e) {
>+    do_check_eq(getOpenedDatabase().lastError, Ci.mozIStorageError.CONSTRAINT);
>+  }
>+  do_check_eq(Ci.mozIStorageStatement.MOZ_STORAGE_STATEMENT_READY, stmt.state);
>+  // Should succeed without needing to reset the statement manually
>+  stmt.finalize();
Did finalize actually fail before your change?  I don't think it would have (but executing another statement would have).

>-             test_getColumnDecltype];
>+             test_getColumnDecltype, test_failed_execute];
nit: place it on its own line with a trailing comma please (I know the rest of them are not like that, but moving forward, that's how it should be).

r=sdwilsh with comments addressed.

Comment 3

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/47b178e78791

Comment 4

[LegNeato]

This was mistakenly landed on .7. We have decided to keep it as-is though, as it is low-risk.