Closed Bug 341137 Opened 18 years ago Closed 17 years ago

Upgrade to sqlite 3.3.17

Categories

(Toolkit :: Storage, defect, P1)

Product:
Toolkit
Component:
Storage
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9alpha6

People

(Reporter: gkw, Assigned: sdwilsh)

References

Details

Attachments

(1 file, 5 obsolete files)

actually changes v1.0
6.26 KB, patch
vlad
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: 

Just released @ http://www.sqlite.org/

Reproducible: Always
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
BeOS compatibility is not yet incorporated in sqlite trunk.  Suspect OS/2 may be impacted also, as we both were by previous update to 3.3.5.

changed to 3.3.7, released 12 aug 06
Summary: Upgrade to sqlite 3.3.6 → Upgrade to sqlite 3.3.7
Summary: Upgrade to sqlite 3.3.7 → Upgrade to sqlite 3.3.8
I'll take this one.
Assignee: vladimir → thunder
Unzip patch and apply (-p0) from mozilla/db/sqlite3/src.

Tested on a palces-enabled build on OS X.
Attachment #243384 - Flags: first-review?(vladimir)
(In reply to comment #4)
> Created an attachment (id=243384) [edit]
> Patch to upgrade to sqlite 3.3.8 (gzipped)
> 
> Unzip patch and apply (-p0) from mozilla/db/sqlite3/src.
> 
> Tested on a palces-enabled build on OS X.
> 

Applying this patch breaks BeOS builds.  We'll likely have to reapply BeOS-specific patches every time we update sqlite, since sqlite's standard package does not include BeOS support.

I have identified additional changes needed to support BeOS.  Is it better to submit a supplemental patch to apply on top of the first, or a completely new diff?
(In reply to comment #3)
> I'll take this one.
> 

Please advise on whether I should submit BeOS changes as a supplemental patch to be applied after the 3.3.8 update, or as a whole new patch.  I'll do whatever works best; I'd just like BeOS builds to not break once this change is committed to the trunk.  Thanks in advance!
Doug,

I can't test your changes either way, so it doesn't really make much of a difference to me.  But I'd lean towards a separate patch, so that it can be looked at separately (since the 3.3.8 one is large, it'd get lost in the noise).
(In reply to comment #8)
> ...lean towards a separate patch...

We'll submit this way, then.  While I have a buildable version based on our previous os_beos.c, it appears there have been enough changes to os_unix.c (on which our code is based) that I've asked another BeOS dev to help comb through and incorporate all the relevant changes.  Please bear with us a few more days.  Is there a target date for committing the upgrade to 3.3.8?


(In reply to comment #9)
> Is there a target date for committing the upgrade to 3.3.8?

As soon as possible; it will probably happen Monday.


(In reply to comment #10)
> (In reply to comment #9)
> > Is there a target date for committing the upgrade to 3.3.8?
> 
> As soon as possible; it will probably happen Monday.
> 
I'll post revisions sufficient to keep BeOS building, then.  We may need to file a subsequent bug to wrap in additional fixes to BeOS-specific code (os_beos.c)  Thanks for the heads-up!


Why have the beos changes not made it into upstream sqlite yet?

This patch is required to maintain BeOS compatibility.
applying main 3.3.8 update removes BeOS changes already incorporated in os.h.  This patch puts back BeOS changes.
Attachment #244991 - Attachment description: documentation change to initial patch now attributes work to contributors by their real names. → incorporates 3.3.8 changes to os_unix .c into os_beos.c Attributes work to contributors by their real names.
(In reply to comment #12)
> Why have the beos changes not made it into upstream sqlite yet?
> 
Mainstream sqlite does not support BeOS.  So far, we've not been able to garner enough interest from the sqlite core team to incorporate our code, though there is a bug open with the sqlite folks (http://www.sqlite.org/cvstrac/tktview?tn=1791) but we don't have a dev dedicated to completing the integration at this time.
Attached patch updated patch for Makefile.in (obsolete) — — Splinter Review 
This updated patch compiles in the required files for doing full text searches in sqlite.
Attachment #252551 - Flags: first-review?(vladimir)
Comment on attachment 244742 [details] [diff] [review]
puts BeOS changes back into os.h after applying main 3.3.8 update

please review.  Would like to commit this at the same time as 3.3.8.
Attachment #244742 - Flags: first-review?(vladimir)
Comment on attachment 244991 [details] [diff] [review]
incorporates 3.3.8 changes to os_unix .c into os_beos.c  Attributes work to contributors by their real names.

please review.  Would like to commit this at the same time as 3.3.8.
Attachment #244991 - Flags: first-review?(vladimir)
Just wondering about attachment 244742 [details] [diff] [review]: why is the fts module needed for SQLite in Mozilla? I don't think any other platform builds that by default, neither for the in-tree-SQLite nor the upstream version, see http://www.sqlite.org/cvstrac/fileview?f=sqlite/Makefile.in&v=1.162 for the latter.
Btw, SQLite is now at 3.3.12...
(In reply to comment #20)

We expect to find FTS useful in a variety of situations, such as for searching through bookmarks, for example.  Extension authors would also be able to put FTS to good use.

It's true that SQLite is already at 3.3.12, though.  Vlad, should I update this bug for 3.3.12?
Will the patches break if the SQLite request is changed to 3.3.12?
(In reply to comment #22)
> Will the patches break if the SQLite request is changed to 3.3.12?
> 
Gary, the BeOS patches may break, or they may apply with a little work.  Since there's no sqlite BeOS version at the moment, the only way to know is to try.  But it makes sense to bring the trunk up to the latest version of sqlite if we're going to go to the trouble of changing.

The latest version is now 3.3.13...

Btw, the extra comment in db/sqlite3/README.MOZILLA about OS/2 was obsolete, as we have added SQLite support for OS/2 upstream long ago. So I just removed it.
The version history <http://www.sqlite.org/changes.html> claims 3.3.13 includes "bug fixes in fts1 and fts2 modules," although it doesn't specify what they are.
The more detailed changes can be seen from the timeline at <http://www.sqlite.org/cvstrac/timeline>. This
http://www.sqlite.org/cvstrac/timeline?d=16&e=2007-Feb-13&c=2&px=&s=9&dm=1&dt=1&x=1&m=1
should show you the detailed checkins between 3.3.12 and 3.3.13... There you can see that indeed checkin [3631] did quite a few changes to the FTS code.
Attachment #243384 - Flags: first-review?(vladimir) → review?(vladimir)
Attachment #252551 - Flags: first-review?(vladimir) → review?(vladimir)
Attachment #244742 - Flags: first-review?(vladimir) → review?(vladimir)
Attachment #244991 - Flags: first-review?(vladimir) → review?(vladimir)
Places needs this for Fx3, setting target milestone to A6
Flags: blocking1.9?
Priority: -- → P1
Target Milestone: --- → mozilla1.9alpha6
fixing title to not be version-specific, as the sqlite version has changed many times over the life of this bug ;)
Summary: Upgrade to sqlite 3.3.8 → Upgrade to latest version of sqlite
Blocks: 342915
Blocks: 380345
it's now 3.3.17. :)
http://hg.sandmill.org/mozilla-patches/raw-file/9ca83aa29b7e/341137-sqlite3-upgrade.patch
(too big to attach to the bug, even compressed)

wip.  I'm clobbering now so haven't been able to test really - I just know it builds ;)

This version uses the new 'amalgamation' sqlite release which is all in one file.  See http://sqlite.org for more info about it.

BeOS patches not ported over, since they are nontrivial and I have no way of testing them.  Doug, are those upstream now?
Is the amalgamation that much of a win even when statically linking?
(In reply to comment #30)
<snip> 
> BeOS patches not ported over, since they are nontrivial and I have no way of
> testing them.  Doug, are those upstream now?
> 
Unfortunately not.  Placing them upstream would entail finding a maintainer for the BeOS additions to sqlite and convincing the sqlite folks to include our changes.  Firefox is, today, the only active user of sqlite in the BeOS community.
Depends on: 384454
Blocks: 384526
(In reply to comment #31)
> Is the amalgamation that much of a win even when statically linking?

According to the sqlite page when the released it:
If you recompile  the amalgamation using GCC option -O3 (the precompiled binaries use -O2) you may see performance improvements of 35% or more over version 3.3.13 depending on your workload.
Attachment #243384 - Flags: review?(vladimir)
Attachment #244742 - Flags: review?(vladimir)
Attachment #244991 - Flags: review?(vladimir)
Attachment #252551 - Flags: review?(vladimir)
Blocks: 365166
It looks like we should be able to compile with -O3 for sqlite too.  An example where we already do this is here:
http://mxr.mozilla.org/seamonkey/source/config/mkdepend/Makefile.in#50
This patch is only the actually changes to the code we use.  The patch that has everything is here:
http://files.shawnwilsher.com/2007/6/18/341137.patch

Mano said it'd be fine to get the preloading working after the fact, which is why it is commented out.
Assignee: thunder → sdwilsh
Attachment #243384 - Attachment is obsolete: true
Attachment #244742 - Attachment is obsolete: true
Attachment #244991 - Attachment is obsolete: true
Attachment #252551 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #268896 - Flags: review?(vladimir)
Probably you have seen that already, but I still wanted to note that SQLite 3.4.0 has been released yesterday. The announcement says "This release fixes two separate bugs either of which can lead to database corruption. Upgrading is strongly recommended." and points to
   http://www.sqlite.org/cvstrac/wiki?p=CorruptionFollowingBusyError
and
   http://www.sqlite.org/cvstrac/tktview?tn=2418
for details on the corruption. I don't know the storage code so maybe these are unlikely to happen...
Also there are two version of sqlite in the tree (NSS just added their own (...) see bug 217538)
We cannot upgrade to the newest version of sqlite until we fix our async io file.  We are using a function that they have now declared static. :(

The previously posted patch does get us up to the previously latest version.
Opening a new bug for the next upgrade as that will have new dependencies.  Also opening a new bug to re-enable preloading.

Checking in storage/src/mozStorageConnection.cpp;
new revision: 1.19; previous revision: 1.18
Checking in db/sqlite3/src/Makefile.in;
new revision: 1.20; previous revision: 1.19
Removing db/sqlite3/src/alter.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/analyze.c;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/attach.c;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/auth.c;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/btree.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/btree.h;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/build.c;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/callback.c;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/complete.c;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/config.h;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/date.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/delete.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/experimental.c;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/expr.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/func.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/hash.c;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/hash.h;
new revision: delete; previous revision: 1.2
Removing db/sqlite3/src/insert.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/keywordhash.h;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/legacy.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/main.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/md5.c;
new revision: delete; previous revision: 1.1
Removing db/sqlite3/src/opcodes.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/opcodes.h;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/os.c;
new revision: delete; previous revision: 1.1
Removing db/sqlite3/src/os.h;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/os_beos.c;
new revision: delete; previous revision: 1.1
Removing db/sqlite3/src/os_common.h;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/os_os2.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/os_os2.h;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/os_unix.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/os_unix.h;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/os_win.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/os_win.h;
new revision: delete; previous revision: 1.1
Removing db/sqlite3/src/pager.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/pager.h;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/parse.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/parse.h;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/pragma.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/prepare.c;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/printf.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/random.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/select.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/shell.c;
new revision: delete; previous revision: 1.7
Checking in db/sqlite3/src/sqlite3.c;
initial revision: 1.1
Checking in db/sqlite3/src/sqlite3.h;
new revision: 1.8; previous revision: 1.7
Checking in db/sqlite3/src/sqlite3file.h;
new revision: 1.3; previous revision: 1.2
Removing db/sqlite3/src/sqliteInt.h;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/table.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/tclsqlite.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/test_async.c;
new revision: delete; previous revision: 1.1
Removing db/sqlite3/src/test_server.c;
new revision: delete; previous revision: 1.2
Removing db/sqlite3/src/tokenize.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/trigger.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/update.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/utf.c;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/util.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/vacuum.c;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/vdbe.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/vdbe.h;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/vdbeInt.h;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/vdbeapi.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/vdbeaux.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/vdbefifo.c;
new revision: delete; previous revision: 1.1
Removing db/sqlite3/src/vdbemem.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/where.c;
new revision: delete; previous revision: 1.7
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Blocks: 385066
backing out...windows hates me...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Found our problem.  Actually, it's a sqlite problem.  They were opening tmp files with the flag FILE_FLAG_DELETE_ON_CLOSE, then trying to delete them.  This of course, would fail.  Our code just does a NS_NOTREACHED if there's some kind of error (which, arguable might need to be worked on a bit; see Bug 385262).  This was causing the shutdown hang with mochitest because the offline resources test uses async IO for the database.  Cookies code does this as well, but I'm guessing those tests don't run on the tinderboxen.  cc'ing robcee to find out about that.

Filed Ticket 2441 (http://www.sqlite.org/cvstrac/tktview?tn=2441) upstream for the sqlite people.
Take 2

Checking in storage/src/mozStorageConnection.cpp;
new revision: 1.21; previous revision: 1.20
Checking in db/sqlite3/src/Makefile.in;
new revision: 1.22; previous revision: 1.21
Removing db/sqlite3/src/alter.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/analyze.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/attach.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/auth.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/btree.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/btree.h;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/build.c;
new revision: delete; previous revision: 1.12
Removing db/sqlite3/src/callback.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/complete.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/config.h;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/date.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/delete.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/experimental.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/expr.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/func.c;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/hash.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/hash.h;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/insert.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/keywordhash.h;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/legacy.c;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/main.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/md5.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/opcodes.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/opcodes.h;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/os.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/os.h;
new revision: delete; previous revision: 1.12
Removing db/sqlite3/src/os_beos.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/os_common.h;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/os_os2.c;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/os_os2.h;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/os_unix.c;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/os_unix.h;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/os_win.c;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/os_win.h;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/pager.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/pager.h;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/pragma.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/prepare.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/printf.c;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/random.c;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/select.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/shell.c;
new revision: delete; previous revision: 1.9
Checking in db/sqlite3/src/sqlite3.c;
new revision: 1.3; previous revision: 1.2
Checking in db/sqlite3/src/sqlite3.h;
new revision: 1.10; previous revision: 1.9
Checking in db/sqlite3/src/sqlite3file.h;
new revision: 1.5; previous revision: 1.4
Removing db/sqlite3/src/sqliteInt.h;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/table.c;
new revision: delete; previous revision: 1.5
Removing db/sqlite3/src/tclsqlite.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/test_async.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/test_server.c;
new revision: delete; previous revision: 1.4
Removing db/sqlite3/src/tokenize.c;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/trigger.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/update.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/utf.c;
new revision: delete; previous revision: 1.6
Removing db/sqlite3/src/util.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/vacuum.c;
new revision: delete; previous revision: 1.7
Removing db/sqlite3/src/vdbe.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/vdbe.h;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/vdbeapi.c;
new revision: delete; previous revision: 1.13
Removing db/sqlite3/src/vdbeaux.c;
new revision: delete; previous revision: 1.11
Removing db/sqlite3/src/vdbefifo.c;
new revision: delete; previous revision: 1.3
Removing db/sqlite3/src/vdbemem.c;
new revision: delete; previous revision: 1.9
Removing db/sqlite3/src/where.c;
new revision: delete; previous revision: 1.9
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Missed three files

Removing db/sqlite3/src/parse.c;
new revision: delete; previous revision: 1.10
Removing db/sqlite3/src/parse.h;
new revision: delete; previous revision: 1.8
Removing db/sqlite3/src/vdbeInt.h;
new revision: delete; previous revision: 1.9
So sqlite was updated to 3.3.17, right?
Would be nice to update that version number here as well:
http://lxr.mozilla.org/mozilla/source/db/sqlite3/README.MOZILLA#2
Summary: Upgrade to latest version of sqlite → Upgrade to sqlite 3.3.17
I filed Bug 385591 about db/sqlite3/README.MOZILLA.
Flags: in-testsuite-
Blocks: 356310
Status: RESOLVED → VERIFIED
No longer blocks: 342915
Flags: blocking1.9?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: