387609 - Add check for lastError in test_storage_progresshandler.js

Status: RESOLVED FIXED

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

Description

[Shawn Wilsher :sdwilsh]

Attachment: Simple C testfile

While trying to fix Bug 384526, I started failing a fairly new test case (yay test driven development!).  Upon further investigation, I found that sqlite was not returning the expected result when a progress handler was telling sqlite to abort (sqlite3_prepare did this as expected, but sqlite3_prepare_v2 does not).

Attached is the test case I used to determine this.  I'm filing a bug upstream on this, and will post a link to it momentarily.

Comment 1

[Shawn Wilsher :sdwilsh]

Upstream Bug:
http://www.sqlite.org/cvstrac/tktview?tn=2497

Comment 2

[Shawn Wilsher :sdwilsh]

Upstream FIXED.  Comment from Ticket:
2007-Jul-11 06:37:52 by danielk1977:
When it comes to setting the database error code, sqlite3_step() behaving the same way regardless of whether sqlite3_prepare() or sqlite3_prepare_v2() is used to prepare the query.

So when an error is returned from sqlite3_step() on a statement prepared with _v2(), the "real" error code is returned but the database error code is being set to SQLITE_ERROR. I'm thinking we should change to set the database error code and message to the "real" error code in this case.

Also, note that this test-case is incompatible with version 3.4.0. In prior versions, if the progress-handler returns non-zero, the statement is halted and SQLITE_ABORT is returned. In 3.4.0, this error code is now SQLITE_INTERRUPT.

When we upgrade, we'll have to keep this in mind.

Comment 3

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

This reduces code complexity as well!  yay!

Comment 4

[(not reading, please use [e-mail] instead)]

Shawn, is this the right patch for this bug?

Comment 5

[Shawn Wilsher :sdwilsh]

Comment on attachment 272195 [details] [diff] [review]
v1.0

No - completely wrong bug...

Comment 6

[Shawn Wilsher :sdwilsh]

Attachment: real v1.0

This is all we need to land the new sqlite.

Comment 7

[(not reading, please use [e-mail] instead)]

fix landed on shawn's behalf.

Checking in storage/src/mozStorage.h;
/cvsroot/mozilla/storage/src/mozStorage.h,v  <--  mozStorage.h
new revision: 1.3; previous revision: 1.2
done
Checking in storage/test/unit/test_storage_progresshandler.js;
/cvsroot/mozilla/storage/test/unit/test_storage_progresshandler.js,v  <--  test_
storage_progresshandler.js
new revision: 1.3; previous revision: 1.2
done