Bug 638555: Make the fields in the bugs table that can be NULL work with all

the boolean chart operators
r=mkanat, a=mkanat (module owner)

2 files changed, 64 insertions, 62 deletions

diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 5c12e2ad6..57b8fe748 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -130,6 +130,9 @@ use Storable qw(dclone);
 # Constants #
 #############
 
+# When doing searches, NULL datetimes are treated as this date.
+use constant EMPTY_DATETIME => '1970-01-01 00:00:00';
+
 # This is the regex for real numbers from Regexp::Common, modified to be
 # more readable.
 use constant NUMBER_REGEX => qr/
@@ -264,14 +267,13 @@ use constant OPERATOR_FIELD_OVERRIDE => {
     },
     
     # General Bug Fields
-    alias => {
-        _non_changed => \&_alias_nonchanged,
-    },
-    # We check all attachment fields against this.
-    attachments => MULTI_SELECT_OVERRIDE,
+    alias        => { _non_changed => \&_nullable },
     'attach_data.thedata' => MULTI_SELECT_OVERRIDE,
-    blocked     => MULTI_SELECT_OVERRIDE,
-    bug_group   => MULTI_SELECT_OVERRIDE,
+    # We check all attachment fields against this.
+    attachments  => MULTI_SELECT_OVERRIDE,
+    blocked      => MULTI_SELECT_OVERRIDE,
+    bug_file_loc => { _non_changed => \&_nullable },
+    bug_group    => MULTI_SELECT_OVERRIDE,
     classification => {
         _non_changed => \&_classification_nonchanged,
     },
@@ -318,10 +320,8 @@ use constant OPERATOR_FIELD_OVERRIDE => {
     },
     tag => MULTI_SELECT_OVERRIDE,
     
-    # Custom multi-select fields
-    _multi_select => MULTI_SELECT_OVERRIDE,
-    
     # Timetracking Fields
+    deadline => { _non_changed => \&_deadline },
     percentage_complete => {
         _non_changed => \&_percentage_complete,
     },
@@ -332,6 +332,13 @@ use constant OPERATOR_FIELD_OVERRIDE => {
         _default      => \&_work_time,
     },
     
+    # Custom Fields
+    FIELD_TYPE_FREETEXT, { _non_changed => \&_nullable },
+    FIELD_TYPE_BUG_ID,   { _non_changed => \&_nullable_int },
+    FIELD_TYPE_DATETIME, { _non_changed => \&_nullable_datetime },
+    FIELD_TYPE_TEXTAREA, { _non_changed => \&_nullable },
+    FIELD_TYPE_MULTI_SELECT, MULTI_SELECT_OVERRIDE,
+    FIELD_TYPE_BUG_URLS,     MULTI_SELECT_OVERRIDE,    
 };
 
 # These are fields where special action is taken depending on the
@@ -1669,16 +1676,15 @@ sub do_search_function {
     
     my $operator_field_override = $self->_get_operator_field_override();
     my $override = $operator_field_override->{$actual_field};
+    # Attachment fields get special handling, if they don't have a specific
+    # individual override.
+    if (!$override and $actual_field =~ /^attachments\./) {
+        $override = $operator_field_override->{attachments};
+    }
+    # If there's still no override, check for an override on the field's type.
     if (!$override) {
-        # Multi-select fields get special handling.
-        if ($self->_multi_select_fields->{$actual_field}) {
-            $override = $operator_field_override->{_multi_select};
-        }
-        # And so do attachment fields, if they don't have a specific
-        # individual override.
-        elsif ($actual_field =~ /^attachments\./) {
-            $override = $operator_field_override->{attachments};
-        }
+        my $field_obj = $self->_chart_fields->{$actual_field};
+        $override = $operator_field_override->{$field_obj->type};
     }
     
     if ($override) {
@@ -2444,10 +2450,32 @@ sub _classification_nonchanged {
         "classifications.id", "classifications", $term);
 }
 
-sub _alias_nonchanged {
+sub _nullable {
     my ($self, $args) = @_;
-    $args->{full_field} = "COALESCE(bugs.alias, '')";
-    $self->_do_operator_function($args);
+    my $field = $args->{full_field};
+    $args->{full_field} = "COALESCE($field, '')";
+}
+
+sub _nullable_int {
+    my ($self, $args) = @_;
+    my $field = $args->{full_field};
+    $args->{full_field} = "COALESCE($field, 0)";
+}
+
+sub _nullable_datetime {
+    my ($self, $args) = @_;
+    my $field = $args->{full_field};
+    my $empty = Bugzilla->dbh->quote(EMPTY_DATETIME);
+    $args->{full_field} = "COALESCE($field, $empty)";
+}
+
+sub _deadline {
+    my ($self, $args) = @_;
+    my $field = $args->{full_field};
+    # This makes "equals" searches work on all DBs (even on MySQL, which
+    # has a bug: http://bugs.mysql.com/bug.php?id=60324).
+    $args->{full_field} = Bugzilla->dbh->sql_date_format($field, '%Y-%m-%d');
+    $self->_nullable_datetime($args);
 }
 
 sub _owner_idle_time_greater_less {
diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm
index a5ec6e2b1..353f9a3bf 100644
--- a/xt/lib/Bugzilla/Test/Search/Constants.pm
+++ b/xt/lib/Bugzilla/Test/Search/Constants.pm
@@ -190,27 +190,6 @@ use constant SUBSTR_NO_FIELD_ADD => FIELD_TYPE_DATETIME, qw(
 # See the KNOWN_BROKEN constant for a general description of these
 # "_BROKEN" constants.
 
-# Certain fields fail all the "negative" search tests:
-#
-# bug_file_loc can be NULL, so it gets missed by the normal
-# notequals search.
-#
-# deadline notequals does not find bugs that lack deadlines
-#
-# setters notequal doesn't find bugs that fully lack flags.
-# (maybe this is OK?)
-#
-# requestees.login_name doesn't find bugs that fully lack requestees.
-use constant NEGATIVE_BROKEN => (
-    bug_file_loc => { contains => [5] },
-    deadline     => { contains => [5] },
-    # Custom fields are busted because they can be NULL.
-    FIELD_TYPE_FREETEXT, { contains => [5] },
-    FIELD_TYPE_BUG_ID,   { contains => [5] },
-    FIELD_TYPE_DATETIME, { contains => [5] },
-    FIELD_TYPE_TEXTAREA, { contains => [5] },
-);
-
 # Shared between greaterthan and greaterthaneq.
 #
 # As with other fields, longdescs greaterthan matches if any comment
@@ -227,9 +206,8 @@ use constant GREATERTHAN_BROKEN => (
 
 # allwords and allwordssubstr have these broken tests in common.
 #
-# allwordssubstr on longdescs fields matches against a single comment,
-# instead of matching against all comments on a bug. Same is true
-# for cc.
+# allwordssubstr on cc fields matches against a single cc,
+# instead of matching against all ccs on a bug.
 use constant ALLWORDS_BROKEN => (
     cc        => { contains => [1] },
 );
@@ -238,7 +216,6 @@ use constant ALLWORDS_BROKEN => (
 #
 # flagtypes.name doesn't match bugs without flags.
 use constant NOWORDS_BROKEN => (
-    NEGATIVE_BROKEN,
     'flagtypes.name' => { contains => [5] },
 );
 
@@ -290,9 +267,6 @@ use constant KNOWN_BROKEN => {
     "equals-%group.<1-bug_group>%" => {
         commenter => { contains => [1,2,3,4,5] },
     },
-    notequals    => { NEGATIVE_BROKEN },
-    notsubstring => { NEGATIVE_BROKEN },
-    notregexp    => { NEGATIVE_BROKEN },
 
     greaterthan   => { GREATERTHAN_BROKEN },
     greaterthaneq => { GREATERTHAN_BROKEN },
@@ -386,13 +360,7 @@ use constant KNOWN_BROKEN => {
 # These are fields that are broken in the same way for pretty much every
 # NOT test that is broken.
 use constant COMMON_BROKEN_NOT => (
-    "bug_file_loc"            => { contains => [5] },
-    "deadline"                => { contains => [5] },
     "flagtypes.name"          => { contains => [5] },
-    FIELD_TYPE_BUG_ID,           { contains => [5] },
-    FIELD_TYPE_DATETIME,         { contains => [5] },
-    FIELD_TYPE_FREETEXT,         { contains => [5] },
-    FIELD_TYPE_TEXTAREA,         { contains => [5] },
 );
 
 # Common BROKEN_NOT values for the changed* fields.
@@ -569,6 +537,8 @@ use constant LESSTHAN_OVERRIDE => (
     qa_contact        => { contains => [1,5] },
     resolution        => { contains => [1,5] },
     status_whiteboard => { contains => [1,5] },
+    FIELD_TYPE_TEXTAREA, { contains => [1,5] },
+    FIELD_TYPE_FREETEXT, { contains => [1,5] },
 );
 
 # The mandatorily-set fields have values higher than <1>,
@@ -754,7 +724,7 @@ use constant TESTS => {
           override => {
               # A lot of these contain bug 5 because an empty value is validly
               # less than the specified value.
-              bug_file_loc => { value => 'http://2-' },
+              bug_file_loc => { value => 'http://2-', contains => [1,5] },
               see_also     => { value => 'http://2-' },
               'attachments.mimetype' => { value => 'text/x-2-' },
               blocked   => { value => '<4-id>', contains => [1,2] },
@@ -770,13 +740,13 @@ use constant TESTS => {
               everconfirmed            => { value => 1, contains => [2,3,4,5] },
               creation_ts => { value => '2037-01-02', contains => [1,5] },
               delta_ts    => { value => '2037-01-02', contains => [1,5] },
-              deadline    => { value => '2037-02-02' },
+              deadline    => { value => '2037-02-02', contains => [1,5] },
               remaining_time => { value => 10, contains => [1,5] },
               percentage_complete => { value => 11, contains => [1,5] },
               longdesc => { value => '2-', contains => [1,5] },
               work_time => { value => 1, contains => [5] },
-              FIELD_TYPE_BUG_ID, { value => '<2>' },
-              FIELD_TYPE_DATETIME, { value => '2037-03-02' },
+              FIELD_TYPE_BUG_ID, { value => '<2>', contains => [1,5] },
+              FIELD_TYPE_DATETIME, { value => '2037-03-02', contains => [1,5] },
               LESSTHAN_OVERRIDE,
           }
         },
@@ -792,14 +762,18 @@ use constant TESTS => {
               'longdescs.count'        => { value => 2, contains => [2,3,4,5] },
               'longdescs.isprivate'    => { value => -1, contains => [] },
               everconfirmed            => { value => 0, contains => [2,3,4,5] },
-              blocked   => { contains => [1,2] },
-              dependson => { contains => [1,3] },
+              bug_file_loc   => { contains => [1,5] },
+              blocked        => { contains => [1,2] },
+              deadline       => { contains => [1,5] },
+              dependson      => { contains => [1,3] },
               creation_ts    => { contains => [1,5] },
               delta_ts       => { contains => [1,5] },
               remaining_time => { contains => [1,5] },
               longdesc       => { contains => [1,5] },
               percentage_complete => { contains => [1,5] },
               work_time => { value => 1, contains => [1,5] },
+              FIELD_TYPE_BUG_ID, { contains => [1,5] },
+              FIELD_TYPE_DATETIME, { contains => [1,5] },
               LESSTHAN_OVERRIDE,
           },
         },