Advertisement:

Author Topic: ERROR Coalesce  (Read 27024 times)

Offline habakuk

  • Semi-Newbie
  • *
  • Posts: 85
ERROR Coalesce
« on: February 13, 2009, 02:44:37 PM »
Hi all. Using rc1, but having seen this probelm in b4 too, I get this Error message, on step 2 of the newsletter functionality:

ERROR: COALESCE types integer and boolean cannot be matched
File: blah/Sources/ManageNews.php
Line: 365

Using postgreSQL and just one mod (Order Sticky Topics), I get the error after selecting "News and Newsletters" > "Newsletters" and selecting any (or none) of the groups to receive a newsletter and posting the form ("Next" button).

In ManageNews.php > ComposeMailing(), the line:

         AND (COALESCE(bg.expire_time, 1=1) OR (bg.expire_time > {int:current_time}))',

seems to throw up.

Any hint on that issue?

TIA

Offline DerEineDa

  • Semi-Newbie
  • *
  • Posts: 35
Re: ERROR Coalesce
« Reply #1 on: February 13, 2009, 07:05:04 PM »
Not using PostgreSQL, but I guess you could try to change that line to:

AND (bg.expire_time IS NULL OR (bg.expire_time > {int:current_time}))',
« Last Edit: February 13, 2009, 07:34:40 PM by DerEineDa »

Offline DerEineDa

  • Semi-Newbie
  • *
  • Posts: 35
Re: ERROR Coalesce
« Reply #2 on: February 13, 2009, 07:37:27 PM »
Thinking a bit further... what purpose has the part "AND (COALESCE(bg.expire_time, 1=1) OR" anyway? This part will always evaluate to "true"! Coalesce will return the first not-null-value of the arguments. If "bg.expire_time" is not NULL, it will be returned, and MySQL will evaluate this part to TRUE, because the value is different from zero.

But if "bg.expire_time" is NULL, then "1=1" will be returned and this is also TRUE. No matter what happens, it's always like there would be written "...AND (TRUE OR...)". Taking this even further, you can see that the left side of this OR-operator is always TRUE, which means that the whole part after the AND will always be TRUE.

This means, that this:

Code: [Select]
WHERE (bg.cannot_access = {int:cannot_access} OR bg.cannot_login = {int:cannot_login})
AND (COALESCE(bg.expire_time, 1=1) OR bg.expire_time > {int:current_time})',
is semantically exactly the same as
Code: [Select]
WHERE (bg.cannot_access = {int:cannot_access} OR bg.cannot_login = {int:cannot_login})
AND TRUE,
. The "AND TRUE" at the end can be omitted.

As far as I understand this, what the programmers wanted to have is this:

Code: [Select]
SELECT DISTINCT mem.id_member
FROM {db_prefix}ban_groups AS bg
INNER JOIN {db_prefix}ban_items AS bi ON (bg.id_ban_group = bi.id_ban_group)
INNER JOIN {db_prefix}members AS mem ON (bi.id_member = mem.id_member)
WHERE (bg.cannot_access = {int:cannot_access} OR bg.cannot_login = {int:cannot_login})
AND (bg.expire_time IS NULL OR bg.expire_time > {int:current_time})

This would get a list of all currently banned member_id. According to the comment inside this file this is exactly what this query should return.

This bug can be found two times in the file ManageNews.php.
« Last Edit: February 13, 2009, 07:40:31 PM by DerEineDa »

Offline habakuk

  • Semi-Newbie
  • *
  • Posts: 85
Re: ERROR Coalesce
« Reply #3 on: February 13, 2009, 08:31:02 PM »
Again, thanks much for the pointer. After reading about Coalesce, I didn't understand what is being tested there, as it indeed always returns true. But still being pretty new to smf, I usually expect it's me not understanding the code ... :)

And when trying to figure what goes on there, I removed the coalesce part and all went well...

Your code change certainly makes sense to me.

Now... is this bug already filed/known?

cheers
®

Offline DerEineDa

  • Semi-Newbie
  • *
  • Posts: 35
Re: ERROR Coalesce
« Reply #4 on: February 13, 2009, 08:37:19 PM »
I don't know if this bug is already known. Hope someone responsible will read this :)

I am quite sure that what I wrote is correct, because I am very confident in my programming/database skills ;)

Offline habakuk

  • Semi-Newbie
  • *
  • Posts: 85
Re: ERROR Coalesce
« Reply #5 on: February 14, 2009, 07:25:47 PM »
Thanks for the bugreport!

cheers
®


Offline karlbenson

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 15,629
  • Gender: Male
    • @mortonssols on Twitter
    • Criminal Solicitors
Re: ERROR Coalesce
« Reply #6 on: February 15, 2009, 08:16:22 AM »
Thanks for reposting the contents/link to this in the bug reports.

Both isses are now on the bug tracker.

Offline markf

  • Semi-Newbie
  • *
  • Posts: 20
Re: ERROR Coalesce
« Reply #7 on: April 10, 2015, 11:41:12 AM »
i know this is an old topic, but it's still happening in 2.09 on postgres.
Is there a link to a bug tracker where I might be able to upvote this or is it internal?
Either way, any chance it'll get fixed? I can manually do it for now, but hate having to hack stuff and having to track changes during upgrades.

EDIT: ... and in editing the Sources/ManageNews.php it's now obvious the first version of the COALESCE was fixed, but the one on line 367 has same issue.

Offline DerEineDa

  • Semi-Newbie
  • *
  • Posts: 35
Re: ERROR Coalesce
« Reply #8 on: April 10, 2015, 11:52:53 AM »
LOL, I just got a notification about this thread via E-Mail. Funny to be reminded about my old posts like this.

Offline markf

  • Semi-Newbie
  • *
  • Posts: 20
Re: ERROR Coalesce
« Reply #9 on: April 10, 2015, 12:57:33 PM »
Ya, I googled the coalesce problem, hit this thread, and saw the fix, assumed it wasn't in yet, but it is... only there's an almost identical block of the original code about 20 lines later with exactly the same mistake in it causing the same error *sigh*

Offline margarett

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 19,761
  • Gender: Male
Re: ERROR Coalesce
« Reply #10 on: April 10, 2015, 08:24:57 PM »
Could you please attach your ManageNews.php, please?
Se forem conduzir, não bebam. Se forem beber... CHAMEM-ME!!!! :D

Quote
Over 90% of all computer problems can be traced back to the interface between the keyboard and the chair

Offline markf

  • Semi-Newbie
  • *
  • Posts: 20
Re: ERROR Coalesce
« Reply #11 on: April 11, 2015, 05:57:28 AM »
Sure, unchanged version attached as requested.
A diff of the fix is:

Code: [Select]
--- Sources/ManageNews.php.001  2015-04-10 16:45:45.655751299 +0100
+++ Sources/ManageNews.php      2015-04-10 16:49:28.026751555 +0100
@@ -364,7 +364,7 @@
                FROM {db_prefix}ban_items AS bi
                        INNER JOIN {db_prefix}ban_groups AS bg ON (bg.id_ban_group = bi.id_ban_group)
                WHERE (bg.cannot_access = {int:cannot_access} OR bg.cannot_login = {int:cannot_login})
-                       AND (COALESCE(bg.expire_time, 1=1) OR bg.expire_time > {int:current_time})
+                       AND (bg.expire_time IS NULL OR bg.expire_time > {int:current_time})
                        AND bi.email_address != {string:blank_string}',
                array(
                        'cannot_access' => 1,

Offline margarett

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 19,761
  • Gender: Male
Re: ERROR Coalesce
« Reply #12 on: April 11, 2015, 08:48:35 AM »
Thank you. Your fix will be included in the next update ;)
Se forem conduzir, não bebam. Se forem beber... CHAMEM-ME!!!! :D

Quote
Over 90% of all computer problems can be traced back to the interface between the keyboard and the chair

Offline markf

  • Semi-Newbie
  • *
  • Posts: 20
Re: ERROR Coalesce
« Reply #13 on: April 11, 2015, 05:58:33 PM »
well, full credit to @DerEineDa for the original fix, but thanks for taking it in.
might want to check any other "1=1" logic, as they'll all break on postgresql, just haven't been hit yet.

Online Kindred

  • The Mean One
  • Support Specialist
  • SMF Legend
  • *
  • Posts: 55,121
  • Gender: Male
    • Kindred-999 on GitHub
Re: ERROR Coalesce
« Reply #14 on: April 11, 2015, 06:44:09 PM »
1=1 is just bad lazy coding (IMO) anyway


-edit - and yes, I am aware that SMF uses this method throughout several different files and queries.  That does not change my opinion that it should not actually be used and there are better ways to format a WHERE...
« Last Edit: April 13, 2015, 11:28:45 AM by Kindred »
Please do not PM, IM or Email me with support questions.  You will get better and faster responses in the support boards.  Thank you.

Offline margarett

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 19,761
  • Gender: Male
Re: ERROR Coalesce
« Reply #15 on: April 14, 2015, 04:57:07 AM »
might want to check any other "1=1" logic, as they'll all break on postgresql, just haven't been hit yet.
Not ALL 1=1 WHERE clauses will break, or your forum will become unusable :P
Eg: to list the boards you can see being administrator, query_see_board *IS* 1=1 instead of FIND_IN_SET(yabadabadaba)
Se forem conduzir, não bebam. Se forem beber... CHAMEM-ME!!!! :D

Quote
Over 90% of all computer problems can be traced back to the interface between the keyboard and the chair

Offline markf

  • Semi-Newbie
  • *
  • Posts: 20
Re: ERROR Coalesce
« Reply #16 on: April 14, 2015, 05:19:47 AM »
My comment about breaking was everywhere different types are used as in the coalesce case (i.e. "integer and boolean cannot be matched").

I'm completely aware that you can do "where 1=1" to do every row selects, and in a dynamically built query, this is clearly being used.

Though personally, my style of coding would test the conditions in code before building the select and not add a where statement if not required, rather than it falling through in this way. In which case this error wouldn't have occurred and you wouldn't be at the whim of the database vendor. But that's a style thing.

Offline margarett

  • SMF Friend
  • SMF Super Hero
  • *
  • Posts: 19,761
  • Gender: Male
Re: ERROR Coalesce
« Reply #17 on: April 14, 2015, 05:25:18 AM »
My comment about breaking was everywhere different types are used as in the coalesce case (i.e. "integer and boolean cannot be matched").
Ups, my bad, sorry :P
Thank you again ;)
Se forem conduzir, não bebam. Se forem beber... CHAMEM-ME!!!! :D

Quote
Over 90% of all computer problems can be traced back to the interface between the keyboard and the chair