337776 - (bz-sqlite) SQLite Support for Bugzilla

Status: RESOLVED FIXED

(Bugzilla :: Database, enhancement)

Description

[Max Kanat-Alexander]

I thought it might be fun to try to hack in SQLite support for Bugzilla.

It would be nice, for example, for packagers to be able to ship a fully-functioning Bugzilla with a sqllite database included.

The trick is that the only ALTER TABLE commands supported by SQLite are RENAME TABLE and ADD COLUMN.

So altering a column will be quite a trick! :-) (We'll have to copy around the whole table.)

We'll use SQLite 3, at least, since that's the newest, unless there's somehow no barrier to support SQLite 2.

Comment 1

[Frédéric Buclin]

SQLite also doesn't support foreign keys, which is a problem in Bugzilla 4.0.

Comment 2

[mackyle]

As of SQLite version 3.6.19, SQLite supports enforcing foreign key constraints.  They must be enabled, but that can be done with the 'PRAGMA foreign_keys = ON;' statement.

See this link for full information:

http://www.sqlite.org/foreignkeys.html

Comment 3

[Max Kanat-Alexander]

(In reply to comment #2)
> As of SQLite version 3.6.19, SQLite supports enforcing foreign key constraints. [snip]

  That's awesome! Thanks for letting us know. :-)

Comment 4

[Max Kanat-Alexander]

Attachment: Work In Progress

Comment 5

[Max Kanat-Alexander]

Attachment: WIP 2

This gets through checksetup up to the point of creating FKs.

I'm going to have to disable the actual creation of FKs, because SQLite can only add them during CREATE TABLE, so that leads us into technical impossibilities (or heavy difficulties) with the way that we manage FKs. (We would have to do a DROP TABLE as part of adding an FK, which would do all sorts of bad things like act as a DELETE on all rows, triggering all existing FKs, and removing all FKs pointing to the current table.)

Comment 6

[Max Kanat-Alexander]

Attachment: v1

Okay, here's a SQLite driver that is complete enough to run checksetup, create a database, and perform nearly all of the functions of Bugzilla. The only reason that I say "nearly all" is that (a) I haven't tested everything and (b) there are certain small things that have to change about Bugzilla in order for the sqlite driver to pass xt/search.t.

Technically I own every part of the code that's touched, here, and so I don't need review. However, as I work on this it would still be helpful to have a second set of eyes on it, whoever wants to look at it. :-)

Comment 7

[Max Kanat-Alexander]

Attachment: v2

This implements a few more sql_ functions, downrevs the DBD::SQLite requirement to something more like what we actually require, and fixes a few bugs that I found here and there.

Comment 8

[Jeff Hammel]

I would really like to use bugzilla as a issue tracker for (mostly) non-mozilla software on my website (http://k0s.org).  However, because I want to synchronize my various instances via unison, a file-based system (i.e. SQLite) seems a likely necessity.  So this is a big deal blocker to me.

Is there anything I can do to test or otherwise help?

Comment 9

[Max Kanat-Alexander]

Hey! Yeah, actually, testing the existing patch with the current Bugzilla trunk would be awesome. I don't think you should use it in production, because you can't upgrade to any later release, currently--attempting to do so will probably corrupt the database in a way that will be difficult to fix (even though it might appear to work okay).

All of the features in the Bugzilla UI and WebService should work properly with the current patch, though, and you should be able to run through the install procedure fully.

Comment 10

[Frédéric Buclin]

Comment on attachment 481471 [details] [diff] [review]
v2

>=== modified file 'xt/lib/Bugzilla/Test/Search/Constants.pm'

>-        operator_ok => [qw(allwordssubstr anywordssubstr casesubstring
>+        operator_ok => [qw(allwords allwordssubstr anywordssubstr casesubstring
>                            changedbefore changedafter greaterthan greaterthaneq
>                            lessthan lessthaneq notregexp notsubstring
>                            nowordssubstr regexp substring anywords
>-                           notequals nowords)],
>+                           notequals nowords equals anyexact)],

This change seems unrelated to this bug, and has already been committed earlier this month.

Comment 11

[Max Kanat-Alexander]

(In reply to comment #10)
> This change seems unrelated to this bug, and has already been committed earlier
> this month.

  Yeah, you're right about that.

Comment 12

[Frédéric Buclin]

Comment on attachment 481471 [details] [diff] [review]
v2

>=== modified file 'Bugzilla/Constants.pm'

>+                                             # 3.6.19 is where FKs were added.
>+    sqlite => {db => 'Bugzilla::DB::Sqlite', db_version => '3.6.19',

Nit: the comment should be left aligned, as you did for MySQL.
Shouldn't we require 3.6.22, to fix a problem when using OR in the WHERE clause? See http://www.sqlite.org/news.html#2010_jan_06 for more information. With the complex queries we have in Search.pm, I think this would be a safe decision.



>=== modified file 'Bugzilla/DB/Schema.pm'

>+    # PRIMARY KEY must appear before NOT NULL for SQLite.
>+    $type_ddl .= " PRIMARY KEY" if ($finfo->{PRIMARYKEY});
>     $type_ddl .= " NOT NULL" if ($finfo->{NOTNULL});
>-    $type_ddl .= " PRIMARY KEY" if ($finfo->{PRIMARYKEY});

In my testing, this change was not needed. I would revert this change, if possible, to make sure we don't break MySQL, Pg or Oracle.



>=== added file 'Bugzilla/DB/Schema/Sqlite.pm'

>+        DATETIME =>     'DATETIME',

DATETIME is not a native format in SQLite. Why not directly set it to the right format? Maybe INTEGER would be a better choice than TEXT COLLATE BINARY used below.


>+    # Don't collate DATETIME fields.
>+    if ($def->{TYPE} eq 'DATETIME') {
>+        $ddl =~ s/\bDATETIME\b/text COLLATE BINARY/;
>+    }

I think this would be unneeded with my suggestion above.



>=== modified file 'Bugzilla/Search.pm'

>-        extra => ['attach_id IS NULL'],
>+        extra => ['map_flags.attach_id IS NULL'],

Is this change related to this bug?



>=== modified file 'xt/lib/Bugzilla/Test/Search/Constants.pm'

Reminder: unrelated to this bug.


Otherwise looks good and works fine, based on my smoketests. :) r=LpSolit, but please answer or address my comments above.

Comment 13

[Max Kanat-Alexander]

(In reply to comment #12)
> Shouldn't we require 3.6.22, to fix a problem when using OR in the WHERE
> clause? 

  Ah, probably. I'll have to make sure that that version comes with that DBD::SQLite.


> Maybe INTEGER would be a better choice than TEXT COLLATE BINARY used
> below.

  No, INTEGER would require massive re-writes of almost every piece of code in Bugzilla that deals with dates. TEXT is the right type for ISO8601 dates, which sort properly as text.

> >-        extra => ['attach_id IS NULL'],
> >+        extra => ['map_flags.attach_id IS NULL'],
> 
> Is this change related to this bug?

  Yes, SQLite complains about it and other databases don't, although some of them probably should.

Comment 14

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified Bugzilla/Constants.pm
modified Bugzilla/DB.pm
modified Bugzilla/Search.pm
modified Bugzilla/DB/Schema.pm
added Bugzilla/DB/Sqlite.pm
added Bugzilla/DB/Schema/Sqlite.pm
modified Bugzilla/Install/Filesystem.pm
modified template/en/default/setup/strings.txt.pl
Committed revision 7578.

Comment 15

[Frédéric Buclin]

Attachment: doc, v1

Comment 16

[:glob ✱]

(In reply to Frédéric Buclin from comment #15)
> Created attachment 560831 [details] [diff] [review]
> doc, v1

i think it's probably worth documenting sqlite's lack of concurrent writing support (see http://sqlite.org/faq.html#q5).  perhaps something like:

"Due to SQLite's concurrency limitations (http://sqlite.org/faq.html#q5) we recommend SQLite only for small and development Bugzilla installations."

Comment 17

[Frédéric Buclin]

Attachment: doc, v2

Comment 18

[:glob ✱]

Comment on attachment 560832 [details] [diff] [review]
doc, v2

looks good, r=glob

Comment 19

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified docs/en/xml/installation.xml
Committed revision 7963.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/
modified docs/en/xml/installation.xml
Committed revision 7931.

Comment 20

[Frédéric Buclin]

Added to relnotes in bug 713346.