Allow Bugzilla::DB sql_regexp/sql_not_regexp methods to accept string and pattern as arguments

RESOLVED FIXED in Bugzilla 2.22

Status

()

Bugzilla
Database
P3
enhancement
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: Lance Larsh, Assigned: Lance Larsh)

Tracking

2.21
Bugzilla 2.22
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

13 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041119
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041119

Currently the abstract methods sql_regexp() and sql_not_regexp() in the
Bugzilla::DB drivers do not take any arguments, and return only the given
database driver's regexp operator name.  Callers construct a regexp query with
the following usage:

   ... "$my_string " . $dbh->sql_regexp . " $my_pattern" ...

This usage assumes the database's regexp operator is a binary infix operator,
but this is not the case for all databases (e.g., in Oracle 10g, it looks like
"REGEXP_LIKE($my_string, $my_pattern)").

Changing the sql_regexp()/sql_not_regexp() methods to take the string and
pattern as arguments would make the database abstraction more generic, allowing
greater flexibility for different types of regexp operators.

Reproducible: Always

Steps to Reproduce:
(Assignee)

Updated

13 years ago
Blocks: 189947

Updated

13 years ago
Assignee: database → lance.larsh
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → 2.21
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

13 years ago
Created attachment 194245 [details] [diff] [review]
Complete sql(_not)_regexp patch

This patch changes the sql_regexp/sql_not_regexp methods to take the string to
be searched and the search pattern as arguments.  The patch also updates all
callers to use the new syntax.

The code change in the "^percentage_complete," block of Bugzilla/Search.pm
seems a little less than ideal, but this seemed to be the simplest way to
change the regexp calls without making huge changes.
Attachment #194245 - Flags: review?(mkanat)

Comment 2

13 years ago
Comment on attachment 194245 [details] [diff] [review]
Complete sql(_not)_regexp patch

Now that sql_regexp() requires arguments, it's not really OK to call it without
arguments in Search.pm (it will throw "uninitialized value" warnings, and it
just seems like not the best idea anyhow).

Also, the POD docs in Bugzilla::DB for sql_regexp need to be updated.

Otherwise, this is an impressively clean patch, particularly for a first
submission.

For the Bugzilla::DB stuff, you'll probably want to ask Joel for review -- he's
better in that area than I am.
Attachment #194245 - Flags: review?(mkanat) → review-
(Assignee)

Comment 3

13 years ago
Created attachment 194327 [details] [diff] [review]
Revised patch w/ POD doc, Search.pm changes

This patch includes everything in the previous patch, plus adds the POD doc
changes and Search.pm changes that Max asked for.
Attachment #194245 - Attachment is obsolete: true
Attachment #194327 - Flags: review?(bugreport)

Comment 4

13 years ago
Comment on attachment 194327 [details] [diff] [review]
Revised patch w/ POD doc, Search.pm changes

I do have to wonder why someone would want to use a regexp for percent
complete.
Attachment #194327 - Flags: review?(bugreport) → review+

Updated

13 years ago
Flags: approval?

Updated

13 years ago
Priority: -- → P3
Target Milestone: --- → Bugzilla 2.22
Flags: approval? → approval+

Comment 5

13 years ago
Oh, and oops, I meant "For the Bugzilla::Search stuff, you'll want to ask Joel."
:-D The Bugzilla::DB stuff is definitely me (or Joel, or various other reviewers
also, of course). :-)

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.434; previous revision: 1.433
done
Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.101; previous revision: 1.100
done
Checking in Bugzilla/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB.pm,v  <--  DB.pm
new revision: 1.64; previous revision: 1.63
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.111; previous revision: 1.110
done
Checking in Bugzilla/DB/Mysql.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v  <--  Mysql.pm
new revision: 1.28; previous revision: 1.27
done
Checking in Bugzilla/DB/Pg.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Pg.pm,v  <--  Pg.pm
new revision: 1.14; previous revision: 1.13
done
Checking in contrib/BugzillaEmail.pm;
/cvsroot/mozilla/webtools/bugzilla/contrib/BugzillaEmail.pm,v  <--  BugzillaEmail.pm
new revision: 1.4; previous revision: 1.3
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.