The announcement CRON is failing with Postgresql
AlexisWilke - October 4, 2008 - 00:13
| Project: | Announcements |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
Description
Hi there,
I was wondering why my announcements would not disappear. The CRON function would use an UPDATE INNER JOIN which I have never seen before (what database are you using?!) It looks like an interesting syntax, but the proper way to work with all databases is to use a sub-select. It probably is slower, but it works with Postgresql too.
In any event, I'm sending a patch.
Thank you.
Alexis
| Attachment | Size |
|---|---|
| announcements-cron-6.x-dev.patch | 1.24 KB |
#1
Thank you for the patch. I see your point. My only defense is that I did not write that code - it came from the original IBM article.
I do have a question before I commit this: Why mark it as a transaction? Publishing new announcements and unpublishing old announcements are really unrelated.
#2
Nancy,
It may not be necessary to have the transaction.
When I saw the code the first time, I thought that if the 1st transaction sets the status to 0, then the 2nd sets it back to 1, it would be better to not change the value at all. The BEGIN + COMMIT prevent any other processes from seeing the status as being 0 if in the end it should be 1.
I guess it is unlikely to happen... 8-) But I should have added one more thing:
$now = time();
then use $now in both statement instead of time(). Because the way it is now, the time changes between each call...
You could have the same problem in many places, actually. Thinking of it, we probably should add an announcements_init() function that saves time() in a global (i.e. $announcements_now) and then only use the global, never use the time() function call again. I have had many problems with that sort of a thing in many different software. 8-)
What do you think?
I'm still wondering about that INNER JOIN within the UPDATE table declaration. It may be a mysql statement because even DB2 does not give that option. At least, according to the following document, even DB2 does not support that syntax...
ftp://ftp.software.ibm.com/ps/products/db2/info/xplatsql/pdf/en_US/cpsql...
(see PDF page 591 and following, document page 573)
Thank you.
Alexis
#3
I have no problem changing the query to a more universal form. It is the first time I've ever actually used a sub-select, so it is interesting .
You are correct about the time. In D7, this will not be a problem because they require PHP 5.2+, which means PHP sets the page start time in $_SERVER['REQUEST_TIME'] (see change). Core recommends switching to that. Unfortunately, D5 and D6 only require 4.3.5+, so I can't use that. However, you are absolutely correct about the time, and making the change you suggest means it will be easier to upgrade the code for D7.
#4
Fix committed on both branches.
#5
Excellent! Using the server request time is a good solution!
Thank you.
Alexis
#6
Automatically closed -- issue fixed for two weeks with no activity.