From add0457ade590235d57f7dbcc0f8ceac02f37617 Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Sun, 12 Jul 2026 20:35:49 -0400 Subject: [PATCH 1/6] Bug 2054397: Test Perl code for vendor-specific SQL --- t/013db_portability.t | 88 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 t/013db_portability.t diff --git a/t/013db_portability.t b/t/013db_portability.t new file mode 100644 index 000000000..eb87d277f --- /dev/null +++ b/t/013db_portability.t @@ -0,0 +1,88 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +use 5.14.0; +use strict; +use warnings; + +use lib qw(. lib local/lib/perl5 t); + +use Support::Files; + +use Test::More; + +my %seen_file; +my @files = grep { $_ ne 't/013db_portability.t' && !$seen_file{$_}++ } + (@Support::Files::testitems, @Support::Files::test_files); + +my @rules = ( + { + token => qr/\bUNIX_TIMESTAMP\s*\(/, + helper => 'sql_date_format', + message => 'use $dbh->sql_date_format(...) instead of UNIX_TIMESTAMP()', + }, + { + token => qr/\bDATE_FORMAT\s*\(/, + helper => 'sql_date_format', + message => 'use $dbh->sql_date_format(...) instead of DATE_FORMAT()', + }, + { + token => qr/\bCONCAT\s*\(/, + helper => 'sql_string_concat', + message => 'use $dbh->sql_string_concat(...) instead of CONCAT()', + }, + { + token => qr/\b(?:POSITION|INSTR|LOCATE)\s*\(/, + helper => 'sql_position/sql_iposition', + message => 'use $dbh->sql_position(...) or $dbh->sql_iposition(...) instead', + }, + { + token => qr/\bGROUP_CONCAT\s*\(/, + helper => 'sql_group_concat', + message => 'use $dbh->sql_group_concat(...) instead of GROUP_CONCAT()', + }, +); + +my @violations; + +foreach my $file (@files) { + next if $file =~ m{^Bugzilla/DB(?:/|\.pm$)}; + + open(my $fh, '<', $file) or do { + push @violations, { file => $file, line => 0, text => 'could not open file' }; + next; + }; + + my $line_number = 0; + while (my $line = <$fh>) { + $line_number++; + next if $line =~ /^\s*#/; + next if $line =~ /^\s*=/; + + foreach my $rule (@rules) { + next unless $line =~ $rule->{token}; + push @violations, { + file => $file, + line => $line_number, + text => $line, + helper => $rule->{helper}, + message => $rule->{message}, + }; + } + } + close($fh); +} + +is(scalar(@violations), 0, 'no raw vendor-specific SQL where a Bugzilla DB helper exists'); + +if (@violations) { + diag(join("\n", map { + sprintf('%s:%d: %s (%s)', $_->{file}, $_->{line}, $_->{message}, $_->{text}) + } @violations)); +} + +done_testing(); From b95c828cd23348fb1500c9c1c09334149ac6d8c8 Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Sun, 12 Jul 2026 20:55:58 -0400 Subject: [PATCH 2/6] Fix the test failures --- Bugzilla/Report/SecurityRisk.pm | 29 ++++++++++++--------- extensions/BMO/lib/Reports/Triage.pm | 4 ++- extensions/BMO/lib/Reports/UserActivity.pm | 4 ++- extensions/BugModal/lib/ActivityStream.pm | 5 ++-- extensions/ProdCompSearch/lib/WebService.pm | 2 +- extensions/UserProfile/lib/Util.pm | 2 +- 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/Bugzilla/Report/SecurityRisk.pm b/Bugzilla/Report/SecurityRisk.pm index 38e284879..2697065eb 100644 --- a/Bugzilla/Report/SecurityRisk.pm +++ b/Bugzilla/Report/SecurityRisk.pm @@ -280,6 +280,7 @@ sub _build_initial_bugs { sub _build_events { my ($self) = @_; + my $dbh = Bugzilla->dbh; return [] if !(@{$self->initial_bug_ids}); my $bug_ids = join ', ', @{$self->initial_bug_ids}; my $start_date = $self->start_date->strftime('%Y-%m-%d %H:%M:%S'); @@ -288,20 +289,22 @@ sub _build_events { bug_id, bug_when, field.name AS field_name, - CONCAT(removed) AS removed, - CONCAT(added) AS added - FROM - bugs_activity - JOIN fielddefs AS field ON fieldid = field.id - JOIN bugs AS bug USING (bug_id) - WHERE - bug_id IN ($bug_ids) - AND field.name IN ('keywords' , 'bug_status') - AND bug_when >= '$start_date' - GROUP BY bug_id , bug_when , field.name - }; + } + . $dbh->sql_string_concat('removed') . qq{ AS removed, + } + . $dbh->sql_string_concat('added') . qq{ AS added + FROM + bugs_activity + JOIN fielddefs AS field ON fieldid = field.id + JOIN bugs AS bug USING (bug_id) + WHERE + bug_id IN ($bug_ids) + AND field.name IN ('keywords' , 'bug_status') + AND bug_when >= '$start_date' + GROUP BY bug_id , bug_when , field.name + }; # Don't use selectall_hashref as it only gets the latest event each bug. - my $result = Bugzilla->dbh->selectall_arrayref($query); + my $result = $dbh->selectall_arrayref($query); my $type = ArrayRef [Tuple [Int, Str, Str, Str, Str]]; $type->assert_valid($result); diff --git a/extensions/BMO/lib/Reports/Triage.pm b/extensions/BMO/lib/Reports/Triage.pm index de8d89f9b..87650ec38 100644 --- a/extensions/BMO/lib/Reports/Triage.pm +++ b/extensions/BMO/lib/Reports/Triage.pm @@ -314,7 +314,9 @@ sub owners { LEFT JOIN attachments AS attachments_1 ON bugs_1.bug_id = attachments_1.bug_id LEFT JOIN flags AS flags_1 ON bugs_1.bug_id = flags_1.bug_id AND (flags_1.attach_id = attachments_1.attach_id OR flags_1.attach_id IS NULL) LEFT JOIN flagtypes AS flagtypes_1 ON flags_1.type_id = flagtypes_1.id - WHERE bugs_1.bug_id = bugs.bug_id AND CONCAT(flagtypes_1.name, flags_1.status) = 'needinfo?'))) + WHERE bugs_1.bug_id = bugs.bug_id AND " + . $dbh->sql_string_concat('flagtypes_1.name', 'flags_1.status') + . " = 'needinfo?'))) AND bugs.component_id = ? GROUP BY bugs.bug_type"); diff --git a/extensions/BMO/lib/Reports/UserActivity.pm b/extensions/BMO/lib/Reports/UserActivity.pm index 75f9be3a3..99f369615 100644 --- a/extensions/BMO/lib/Reports/UserActivity.pm +++ b/extensions/BMO/lib/Reports/UserActivity.pm @@ -182,7 +182,9 @@ sub report { 'longdesc' AS name, longdescs.bug_id, NULL AS attach_id, - DATE_FORMAT(longdescs.bug_when, '%Y-%m-%d %H:%i:%s') AS ts, + " + . $dbh->sql_date_format('longdescs.bug_when', '%Y-%m-%d %H:%i:%s') + . " AS ts, '' AS removed, '' AS added, profiles.login_name, diff --git a/extensions/BugModal/lib/ActivityStream.pm b/extensions/BugModal/lib/ActivityStream.pm index c3fe4d88d..e6530cd32 100644 --- a/extensions/BugModal/lib/ActivityStream.pm +++ b/extensions/BugModal/lib/ActivityStream.pm @@ -358,7 +358,7 @@ sub _add_duplicates_to_stream { my $sth = $dbh->prepare(" SELECT longdescs.who, - UNIX_TIMESTAMP(bug_when), " . $dbh->sql_date_format('bug_when') . ", + " . $dbh->sql_date_format('bug_when') . ", type, extra_data FROM longdescs @@ -368,7 +368,8 @@ sub _add_duplicates_to_stream { "); $sth->execute($bug->id, CMT_HAS_DUPE, CMT_DUPE_OF); - while (my ($who, $time, $when, $type, $dupe_id) = $sth->fetchrow_array) { + while (my ($who, $when, $type, $dupe_id) = $sth->fetchrow_array) { + my $time = date_str_to_time($when); _add_activity_to_stream( $stream, $time, $who, { diff --git a/extensions/ProdCompSearch/lib/WebService.pm b/extensions/ProdCompSearch/lib/WebService.pm index 4b758e97a..6d3a73f9d 100644 --- a/extensions/ProdCompSearch/lib/WebService.pm +++ b/extensions/ProdCompSearch/lib/WebService.pm @@ -207,7 +207,7 @@ sub _build_like_order { my @terms; foreach my $word (split(/[\s,]+/, $query)) { push @terms, - "CONCAT(products.name, components.name) LIKE " + $dbh->sql_string_concat('products.name', 'components.name') . " LIKE " . $dbh->quote('%' . $word . '%') if $word ne ''; } diff --git a/extensions/UserProfile/lib/Util.pm b/extensions/UserProfile/lib/Util.pm index 7c0c3353a..49e2a2bcb 100644 --- a/extensions/UserProfile/lib/Util.pm +++ b/extensions/UserProfile/lib/Util.pm @@ -247,7 +247,7 @@ sub _activity_by_status { AND fieldid = ? GROUP BY added UNION ALL - SELECT CONCAT('RESOLVED/', added) AS status, COUNT(*) AS count + SELECT @{[$dbh->sql_string_concat($dbh->quote('RESOLVED/'), 'added')]} AS status, COUNT(*) AS count FROM bugs_activity WHERE who = ? AND fieldid = ? From 96168c13ab87b0e58f6e99e33c681c1402d6a11e Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Sun, 12 Jul 2026 21:02:21 -0400 Subject: [PATCH 3/6] Revert accidental spacing change --- Bugzilla/Report/SecurityRisk.pm | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Bugzilla/Report/SecurityRisk.pm b/Bugzilla/Report/SecurityRisk.pm index 2697065eb..86857d388 100644 --- a/Bugzilla/Report/SecurityRisk.pm +++ b/Bugzilla/Report/SecurityRisk.pm @@ -293,15 +293,15 @@ sub _build_events { . $dbh->sql_string_concat('removed') . qq{ AS removed, } . $dbh->sql_string_concat('added') . qq{ AS added - FROM - bugs_activity - JOIN fielddefs AS field ON fieldid = field.id - JOIN bugs AS bug USING (bug_id) - WHERE - bug_id IN ($bug_ids) - AND field.name IN ('keywords' , 'bug_status') - AND bug_when >= '$start_date' - GROUP BY bug_id , bug_when , field.name + FROM + bugs_activity + JOIN fielddefs AS field ON fieldid = field.id + JOIN bugs AS bug USING (bug_id) + WHERE + bug_id IN ($bug_ids) + AND field.name IN ('keywords' , 'bug_status') + AND bug_when >= '$start_date' + GROUP BY bug_id , bug_when , field.name }; # Don't use selectall_hashref as it only gets the latest event each bug. my $result = $dbh->selectall_arrayref($query); From 3effc4ef3ff516696a715417661ba8cbee52118a Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Sun, 12 Jul 2026 22:15:26 -0400 Subject: [PATCH 4/6] Test the contents of the scripts directory as well. --- t/013db_portability.t | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/t/013db_portability.t b/t/013db_portability.t index eb87d277f..f44fd6fc5 100644 --- a/t/013db_portability.t +++ b/t/013db_portability.t @@ -12,6 +12,7 @@ use warnings; use lib qw(. lib local/lib/perl5 t); use Support::Files; +use File::Find; use Test::More; @@ -19,6 +20,18 @@ my %seen_file; my @files = grep { $_ ne 't/013db_portability.t' && !$seen_file{$_}++ } (@Support::Files::testitems, @Support::Files::test_files); +my @script_files; +find( + sub { + return if -d; + return unless /\.pl$/; + push @script_files, $File::Find::name; + }, + 'scripts' +); + +push @files, grep { !$seen_file{$_}++ } @script_files; + my @rules = ( { token => qr/\bUNIX_TIMESTAMP\s*\(/, From 6eda1c32dd2b4b22eb3629691223fca0bd63f713 Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Sun, 12 Jul 2026 22:47:57 -0400 Subject: [PATCH 5/6] Add $dbh->sql_date_to_epoch abstraction and suggest it in test --- Bugzilla/DB.pm | 27 ++++++++++++++++++++++++++- Bugzilla/DB/MariaDB.pm | 6 ++++++ Bugzilla/DB/Mysql.pm | 6 ++++++ Bugzilla/DB/Oracle.pm | 6 ++++++ Bugzilla/DB/Pg.pm | 6 ++++++ Bugzilla/DB/Sqlite.pm | 6 ++++++ t/013db_portability.t | 4 ++-- 7 files changed, 58 insertions(+), 3 deletions(-) diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm index 1f0daf051..b784f3a1a 100644 --- a/Bugzilla/DB.pm +++ b/Bugzilla/DB.pm @@ -411,7 +411,7 @@ EOT # List of abstract methods we are checking the derived class implements our @_abstract_methods = qw(new sql_regexp sql_not_regexp sql_limit sql_to_days - sql_date_format sql_date_math bz_explain + sql_date_format sql_date_math sql_date_to_epoch bz_explain sql_group_concat); # This overridden import method will check implementation of inherited classes @@ -2192,6 +2192,31 @@ Formatted SQL for adding or subtracting a date and some amount of time (scalar) =back +=item C + +=over + +=item B + +Outputs SQL syntax for converting a date/time expression to Unix epoch +seconds. + +Abstract method, should be overridden by database specific code. + +=item B + +=over + +=item C<$date> - date or name of date type column (scalar) + +=back + +=item B + +Formatted SQL for Unix epoch seconds (scalar) + +=back + =item C =over diff --git a/Bugzilla/DB/MariaDB.pm b/Bugzilla/DB/MariaDB.pm index 5db175ef2..4289306b0 100644 --- a/Bugzilla/DB/MariaDB.pm +++ b/Bugzilla/DB/MariaDB.pm @@ -228,6 +228,12 @@ sub sql_date_math { return "$date $operator INTERVAL $interval $units"; } +sub sql_date_to_epoch { + my ($self, $date) = @_; + + return "UNIX_TIMESTAMP($date)"; +} + sub sql_iposition { my ($self, $fragment, $text) = @_; return "INSTR($text, $fragment)"; diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm index 214618031..e30f85c0b 100644 --- a/Bugzilla/DB/Mysql.pm +++ b/Bugzilla/DB/Mysql.pm @@ -229,6 +229,12 @@ sub sql_date_math { return "$date $operator INTERVAL $interval $units"; } +sub sql_date_to_epoch { + my ($self, $date) = @_; + + return "UNIX_TIMESTAMP($date)"; +} + sub sql_iposition { my ($self, $fragment, $text) = @_; return "INSTR($text, $fragment)"; diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm index 9e8c16e72..fe0039f4d 100644 --- a/Bugzilla/DB/Oracle.pm +++ b/Bugzilla/DB/Oracle.pm @@ -203,6 +203,12 @@ sub sql_date_math { return "$date $operator $time_sql"; } +sub sql_date_to_epoch { + my ($self, $date) = @_; + + return "TRUNC(($date - DATE '1970-01-01') * 86400)"; +} + sub sql_position { my ($self, $fragment, $text) = @_; return "INSTR($text, $fragment)"; diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm index 7afbee109..4daa79810 100644 --- a/Bugzilla/DB/Pg.pm +++ b/Bugzilla/DB/Pg.pm @@ -155,6 +155,12 @@ sub sql_date_math { return "$date $operator $interval * INTERVAL '1 $units'"; } +sub sql_date_to_epoch { + my ($self, $date) = @_; + + return "CAST(EXTRACT(EPOCH FROM $date) AS BIGINT)"; +} + sub sql_string_concat { my ($self, @params) = @_; diff --git a/Bugzilla/DB/Sqlite.pm b/Bugzilla/DB/Sqlite.pm index bbd9d34e6..ac7a51255 100644 --- a/Bugzilla/DB/Sqlite.pm +++ b/Bugzilla/DB/Sqlite.pm @@ -235,6 +235,12 @@ sub sql_date_math { return "DATETIME($date, '$operator' || $interval || ' $units')"; } +sub sql_date_to_epoch { + my ($self, $date) = @_; + + return "CAST(STRFTIME('%s', $date) AS INTEGER)"; +} + ############### # bz_ methods # ############### diff --git a/t/013db_portability.t b/t/013db_portability.t index f44fd6fc5..32c394bfb 100644 --- a/t/013db_portability.t +++ b/t/013db_portability.t @@ -35,8 +35,8 @@ push @files, grep { !$seen_file{$_}++ } @script_files; my @rules = ( { token => qr/\bUNIX_TIMESTAMP\s*\(/, - helper => 'sql_date_format', - message => 'use $dbh->sql_date_format(...) instead of UNIX_TIMESTAMP()', + helper => 'sql_date_to_epoch', + message => 'use $dbh->sql_date_to_epoch(...) instead of UNIX_TIMESTAMP()', }, { token => qr/\bDATE_FORMAT\s*\(/, From 8ecd408f700c6f37f42ed0dd6347d0ea92e0c569 Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Sun, 12 Jul 2026 22:48:34 -0400 Subject: [PATCH 6/6] Fix new test failures --- extensions/BugModal/lib/ActivityStream.pm | 6 +++--- scripts/nagios_blocker_checker.pl | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions/BugModal/lib/ActivityStream.pm b/extensions/BugModal/lib/ActivityStream.pm index e6530cd32..df76cadd7 100644 --- a/extensions/BugModal/lib/ActivityStream.pm +++ b/extensions/BugModal/lib/ActivityStream.pm @@ -358,7 +358,8 @@ sub _add_duplicates_to_stream { my $sth = $dbh->prepare(" SELECT longdescs.who, - " . $dbh->sql_date_format('bug_when') . ", + " . $dbh->sql_date_to_epoch('bug_when') . ", + " . $dbh->sql_date_format('bug_when') . ", type, extra_data FROM longdescs @@ -368,8 +369,7 @@ sub _add_duplicates_to_stream { "); $sth->execute($bug->id, CMT_HAS_DUPE, CMT_DUPE_OF); - while (my ($who, $when, $type, $dupe_id) = $sth->fetchrow_array) { - my $time = date_str_to_time($when); + while (my ($who, $time, $when, $type, $dupe_id) = $sth->fetchrow_array) { _add_activity_to_stream( $stream, $time, $who, { diff --git a/scripts/nagios_blocker_checker.pl b/scripts/nagios_blocker_checker.pl index 7d04f01f6..da1b154f7 100755 --- a/scripts/nagios_blocker_checker.pl +++ b/scripts/nagios_blocker_checker.pl @@ -183,8 +183,8 @@ $where .= " AND bug_severity IN ($severities)"; } - my $sql = <<"EOF"; - SELECT bug_id, bug_severity, UNIX_TIMESTAMP(bugs.creation_ts) AS ts + my $sql = <<"EOF"; + SELECT bug_id, bug_severity, @{[$dbh->sql_date_to_epoch('bugs.creation_ts')]} AS ts FROM bugs WHERE $where AND COALESCE(resolution, '') = ''