Bug 38003 - We should add FTS3 support to WebKit
Summary: We should add FTS3 support to WebKit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Dumitru Daniliuc
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-22 13:25 PDT by Dumitru Daniliuc
Modified: 2010-05-25 15:42 PDT (History)
9 users (show)

See Also:


Attachments
enable FTS3 (1.46 KB, patch)
2010-05-14 16:08 PDT, Dumitru Daniliuc
beidson: review+
dumi: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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