Closed Bug 387609 Opened 17 years ago Closed 17 years ago

Add check for lastError in test_storage_progresshandler.js

Categories

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

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla1.9beta1

People

(Reporter: sdwilsh, Assigned: sdwilsh)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached file 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.
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.
Depends on: 385067
Attached patch v1.0 (obsolete) — Splinter Review
This reduces code complexity as well! yay!
Attachment #272195 - Flags: review?(sspitzer)
Shawn, is this the right patch for this bug?
Comment on attachment 272195 [details] [diff] [review] v1.0 No - completely wrong bug...
Attachment #272195 - Attachment is obsolete: true
Attachment #272195 - Flags: review?(sspitzer)
Attached patch real v1.0Splinter Review
This is all we need to land the new sqlite.
Attachment #274221 - Flags: review?(sspitzer)
Target Milestone: --- → mozilla1.9beta2
Attachment #274221 - Flags: review?(sspitzer) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Product: Toolkit → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: