313209 - Oracle requires "CASE WHEN" around boolean expressions in the SELECT column list

Status: RESOLVED FIXED

(Bugzilla :: Database, enhancement)

Description

[Lance Larsh]

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

Oracle doesn't allow boolean expressions directly in the column list of a SELECT
statement.  Queries will fail on Oracle if written as:

   SELECT col IS NULL FROM t;

To make this query work on Oracle, it must be written as:

   SELECT CASE WHEN col IS NULL THEN 1 ELSE 0 END FROM t;

Most queries already have the CASE WHEN clause, but there are a handful that don't.

Reproducible: Always

Steps to Reproduce:




Affected queries:
   Bugzilla/Bug.pm:549
   Bugzilla/User.pm:492
   Bugzilla/User.pm:510
   chart.cgi:210
   editproducts.cgi:197
   editproducts.cgi:834
   editproducts.cgi:868
   globals.pl:758
   process_bug.cgi:1668

Comment 1

[Lance Larsh]

(In reply to comment #0)

Hmm, the code has changed quite a bit since my last pull a few days ago.  One of
the offending queries was removed altogether, and the others changed line #s 
(and hopefully no new ones have been introduced).  The list of queries is now:
   Bugzilla/Bug.pm:549
   Bugzilla/User.pm:512
   Bugzilla/User.pm:530
   chart.cgi:210
   editproducts.cgi:628
   editproducts.cgi:660
   globals.pl:737
   process_bug.cgi:1723

Comment 2

[Lance Larsh]

Attachment: Patch to add CASE WHEN clauses

One of the diffs in this patch (i.e., the diff in process_bug.cgi) may fail to
apply depending on whether the patch for Bug 312349 (Attachment 199595 [details] [diff]) has
been checked in or not, since both patches touch that file about 2-3 lines away
from each other.  This patch is based on today's CVS source, which does not yet
have 312349 applied.

This patch addresses all the occurrences I found.  I manually checked all 563
(yikes!) SELECT statements that existed in the Bugzilla code as of a few days
ago, and these were the only ones that were affected at that time.

Comment 3

[Joel Peshkin]

Does this have any significant performance impact on the other databses?  Should
it be a  $dbh->boolean() operation?

Comment 4

[Lance Larsh]

(In reply to comment #3)
> Does this have any significant performance impact on the other databses?

Hmm, I really don't know.  But it does appear that using "CASE WHEN ... THEN 1 
ELSE 0 END" is already done elsewhere in Bugzilla.  In fact, some of the 
queries my patch touches already have a boolean CASE WHEN in other fields of 
the SELECT list.

> Should it be a  $dbh->boolean() operation?

That's not my call, but I see 19 places where "THEN 1 ELSE 0 END" already 
exists.  Is there a reason those 19 places needed the CASE WHEN for the other 
DBs while the other 8 spots I found didn't?  I'm guessing no and that it was 
just an oversight, but I'm no expert on the other DBs.

Comment 5

[Joel Peshkin]

I think that, as long as this is only in the fields returned and not in any
criteria, we should be OK.  That seems to be the case in all of the cases you
have in the patch.  If any of these become "AS alias" fields that feed into a
WHERE condition or get used in a subselect, it could be more of a question.

Comment 6

[Max Kanat-Alexander]

Comment on attachment 200298 [details] [diff] [review]
Patch to add CASE WHEN clauses

This looks fine to me.

HOWEVER: There are several lines that are longer than 80 characters, which
violates our style guide. Those need to be fixed before you check this in.

Request approval once you have that fixed.

Comment 7

[Lance Larsh]

Attachment: Patch v2

This patch reformats long lines, otherwise it's the same as Attachment 200298 [details] [diff]

Comment 8

[Max Kanat-Alexander]

Checking in chart.cgi;
/cvsroot/mozilla/webtools/bugzilla/chart.cgi,v  <--  chart.cgi
new revision: 1.15; previous revision: 1.14
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.103; previous revision: 1.102
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.344; previous revision: 1.343
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.291; previous revision: 1.290
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.99; previous revision: 1.98
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.90; previous revision: 1.89
done