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: database → lance.larsh
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → 2.21
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 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-
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.
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+
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.