Bug 38003

Summary: We should add FTS3 support to WebKit
Product: WebKit Reporter: Dumitru Daniliuc <dumi>
Component: New BugsAssignee: Dumitru Daniliuc <dumi>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aroben, beidson, dglazkov, eric, fishd, michaeln, mjs, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
enable FTS3 beidson: review+, dumi: commit-queue-

Description Dumitru Daniliuc 2010-04-22 13:25:28 PDT
Gears supported FTS2, so I think we should add FTS3 support to WebKit, to make it easier for web developers who used Gears port their apps to WebSQLDatabases.
Comment 1 Darin Fisher (:fishd, Google) 2010-04-23 00:35:10 PDT
Can you comment on why FTS3 should be added instead of FTS2?  Are there compatibility concerns?
Comment 2 Dumitru Daniliuc 2010-04-23 12:11:29 PDT
(In reply to comment #1)
> Can you comment on why FTS3 should be added instead of FTS2?  Are there
> compatibility concerns?

FTS2 has a design problem: calling VACUUM on FTS2 databases could corrupt them. According to Scott, FTS3 is pretty much FTS2 with this problem fixed.

It is unclear if FTS2 databases are compatible with FTS3. We believe they should be, but I haven't tested this. However, WebKit doesn't have FTS2 support, and Chromium's FTS2 support is incomplete at the moment because we don't allow the MATCH operator (bug 37049). So I think it should be OK to treat Chromium's FTS2 support as experimental, and say that full-text search will be officially supported starting with FTS3.
Comment 3 Michael Nordman 2010-04-23 13:03:41 PDT
(In reply to comment #1)
> Can you comment on why FTS3 should be added instead of FTS2?  Are there
> compatibility concerns?

FTS3 is being actively supported by SQLite, FTS2 is not.

FTS2 and has a known bug (around vacuum) that is fixed in FTS3. As I understand it, FTS3 was the fix for that known bug.

There is a data format change, FTS2 code can't read FTS3 data and vice versa.

(Just passing along what I've learned from shess)
Comment 4 Darin Fisher (:fishd, Google) 2010-04-23 21:52:21 PDT
OK, I agree that it would be valuable for FTS3 to be enabled across all ports of WebKit.  /cc @mjs for his opinion.
Comment 5 Brady Eidson 2010-04-24 00:03:08 PDT
I've been discussing this in emails with some of our SQLite gurus at Apple.  I think they think this is fine, as long as we still restrict some things with the authorizer.  fts3_tokenizer has come up by name, there may be others.
Comment 6 Brady Eidson 2010-04-24 00:03:56 PDT
(In reply to comment #5)
> I've been discussing this in emails with some of our SQLite gurus at Apple.  I
> think they think this is fine, as long as we still restrict some things with
> the authorizer.  fts3_tokenizer has come up by name, there may be others.

Sorry - also meant to mention that I'll have all the info they mean to give me by Monday.
Comment 7 Dumitru Daniliuc 2010-04-24 03:21:18 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > I've been discussing this in emails with some of our SQLite gurus at Apple.  I
> > think they think this is fine, as long as we still restrict some things with
> > the authorizer.  fts3_tokenizer has come up by name, there may be others.
> 
> Sorry - also meant to mention that I'll have all the info they mean to give me
> by Monday.

Great! fts2_tokenizer() is currently disabled in DatabaseAuthorizer, because our security experts didn't like it too, and I'm pretty sure they agree that fts3_tokenizer() should be disabled too.
Comment 8 Dumitru Daniliuc 2010-05-04 16:33:00 PDT
Brady, any updates?
Comment 9 Dumitru Daniliuc 2010-05-14 16:08:52 PDT
Created attachment 56118 [details]
enable FTS3
Comment 10 Dumitru Daniliuc 2010-05-25 13:33:20 PDT
Ping.
Comment 11 Brady Eidson 2010-05-25 13:51:30 PDT
Comment on attachment 56118 [details]
enable FTS3

It really sucks that we're fragmenting capabilities like this, but I guess we already did it with fts2.  

That said, no one here sees an inherent problem with fts3.
Comment 12 Dumitru Daniliuc 2010-05-25 15:06:00 PDT
Landed as r60180.
Comment 13 WebKit Review Bot 2010-05-25 15:42:23 PDT
http://trac.webkit.org/changeset/60188 might have broken GTK Linux 32-bit Release