301062 - [PostgreSQL] whine.pl fails when using PostgreSQL 8.0.1

Status: RESOLVED FIXED

(Bugzilla :: Whining, defect)

Description

[Frédéric Buclin]

[root@antares bugzilla-pg]# ./whine.pl
DBD::Pg::st execute failed: called with 1 bind variables when 0 are needed [for
Statement "SELECT CURRENT_DATE() + INTERVAL '? DAY'"] at ./whine.pl line 696
        main::get_next_date(3) called at ./whine.pl line 193


whine.pl, line 696 is:

    # Get a date in whatever format the database will accept
    $sth = $dbh->prepare("SELECT CURRENT_DATE() + " .
                         $dbh->sql_interval('? DAY'));
    $sth->execute($add_days);

Comment 1

[Max Kanat-Alexander]

This once again looks like a DBD::Pg bug, somehow failing to recognize a
placeholder.

Comment 2

[Frédéric Buclin]

(In reply to comment #1)
> This once again looks like a DBD::Pg bug, somehow failing to recognize a
> placeholder.

I have installed DBD::Pg 1.43. Sorry, but 1.44 hasn't been released yet!

Comment 3

[Andrew Dunstan]

a workaround could be to use something like:

  select current_date + ? * interval '1 day'

Note that you must NOT have () after current_date in postgres (a weirdness
mandated by the sql standard).

Comment 4

[A. Karl Kornel]

This might be fixed by bug 304989, which does the date/time computation in Perl instead of SQL.  
There's only one query remaining that uses dates.

Is it OK to have a query of the form "SELECT one_or_more_columns FROM some_table WHERE 
some_datetime_column <= '2005-10-20 12:30:52'"?  If the answer is 'yes' then this bug will probably 
be fixed by bug 304989.

Comment 5

[Frédéric Buclin]

I think this is a blocker for anyone who wants to use PostgreSQL and the whining system, either on 2.20 or tip.

Comment 6

[Max Kanat-Alexander]

OK, I'll see if I can get a chance to take a look at it. If somebody else figures it out, that would be great, too. It does still seem like a DBD::Pg bug.

Comment 7

[Frédéric Buclin]

error message returned on tip:

whine.pl: DBD::Pg::st execute failed: called with 2 bind variables when 1 are needed [for Statement "UPDATE whine_schedules SET run_next = NOW() + INTERVAL '? MINUTE' - INTERVAL 'SECOND(NOW()) SECOND' WHERE id = ?" with ParamValues: ] at ./whine.pl line 611
whine.pl:    main::reset_timer(11) called at ./whine.pl line 289
whine.pl:    main::get_next_event() called at ./whine.pl line 317
whine.pl: DBI::db=HASH(0x8a82548)->disconnect invalidates 1 active statement handle (either destroy statement handles or call finish on them before disconnecting) at Bugzilla.pm line 295.

Comment 8

[Frédéric Buclin]

Attachment: patch, v1

OK, after a long investigation and discussion on #postgresqlfr, I finally reach to fix whine.pl. Now it runs on Pg perfectly. :)

There were several problems:

- CURRENT_DATE() doesn't exist on Pg 8. It must be CURRENT_DATE;
- 'CURRENT_DATE + INTERVAL foo' must be in the query where it will really be used, due to a CAST problem. In other words:

my $foo = $dbh->selectrow_array('SELECT CURRENT_DATE + ' . $dbh->sql_interval('bar'));

$dbh->do('UPDATE baz SET run_when = ? WHERE ...', undef, ($foo, ...));

doesn't work. But:

$dbh->do('UPDATE baz SET run_when = CURRENT_DATE + ' . $dbh->sql_interval('bar') . ' WHERE ...', undef, ....);

works.
- INTERVAL '? DAY' doesn't work on Pg 8. We have to write:
  ? * INTERVAL '1 DAY'


I tested my patch on MySQL 4.1.11, PostgreSQL 7.4.8, 8.0.1 and 8.0.4 successfully.

Comment 9

[Max Kanat-Alexander]

Comment on attachment 201675 [details] [diff] [review]
patch, v1

This *looks* basically good, but I won't have time to test it, and it definitely needs some testing.

Comment 10

[Frédéric Buclin]

Comment on attachment 201675 [details] [diff] [review]
patch, v1

manu, justdave and joel said you could test this one. ;)

(18:32:13) joel: manu's assurances that he has tested it are an input to a reviewer or approver signing off that it is good enough.
(18:32:15) justdave_: yeah, I agree.  I don't mind outside reviewers if it's something specific that we know they have expertise in.

Comment 11

[Frédéric Buclin]

Attachment: backport for 2.20, v1

same patch as for the tip (minor bitrot)

Comment 12

[Frédéric Buclin]

Comment on attachment 201773 [details] [diff] [review]
backport for 2.20, v1

manu, same patch as before, but for 2.20.

Comment 13

[Emmanuel Seyman]

Comment on attachment 201675 [details] [diff] [review]
patch, v1

It works fine for me, both under Pg and under MySQL

Comment 14

[Emmanuel Seyman]

Comment on attachment 201773 [details] [diff] [review]
backport for 2.20, v1

Okay for me.

Comment 15

[Max Kanat-Alexander]

Comment on attachment 201773 [details] [diff] [review]
backport for 2.20, v1

Yeah, I like this. I always wanted to do this this way anyhow.

I suspect that whine.pl neads some re-architecture.

Comment 16

[Max Kanat-Alexander]

Comment on attachment 201675 [details] [diff] [review]
patch, v1

If manu says this works, that's good enough for me.

Comment 17

[Max Kanat-Alexander]

By the way, I'm not setting blocking2.22+ only because I can't -- the trunk is under Dave's control. I'm just the branch guy, and we haven't branched yet.

Comment 18

[A. Karl Kornel]

(In reply to comment #15)
> <<<snip>>>
> I suspect that whine.pl neads some re-architecture.
> 

You'll want to see bug 304989 for that 8-)

Comment 19

[Frédéric Buclin]

tip:

Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.105; previous revision: 1.104
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.92; previous revision: 1.91
done
Checking in whine.pl;
/cvsroot/mozilla/webtools/bugzilla/whine.pl,v  <--  whine.pl
new revision: 1.18; previous revision: 1.17
done
Checking in Bugzilla/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB.pm,v  <--  DB.pm
new revision: 1.68; previous revision: 1.67
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.117; previous revision: 1.116
done
Checking in Bugzilla/Token.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Token.pm,v  <--  Token.pm
new revision: 1.38; previous revision: 1.37
done
Checking in Bugzilla/DB/Mysql.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v  <--  Mysql.pm
new revision: 1.30; previous revision: 1.29
done
Checking in Bugzilla/DB/Pg.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Pg.pm,v  <--  Pg.pm
new revision: 1.16; previous revision: 1.15
done
Checking in contrib/sendunsentbugmail.pl;
/cvsroot/mozilla/webtools/bugzilla/contrib/sendunsentbugmail.pl,v  <--  sendunsentbugmail.pl
new revision: 1.7; previous revision: 1.6
done


2.20:

Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.97.2.1; previous revision: 1.97
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.75.4.5; previous revision: 1.75.4.4
done
Checking in whine.pl;
/cvsroot/mozilla/webtools/bugzilla/whine.pl,v  <--  whine.pl
new revision: 1.13.2.2; previous revision: 1.13.2.1
done
Checking in Bugzilla/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB.pm,v  <--  DB.pm
new revision: 1.56.2.4; previous revision: 1.56.2.3
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.99.2.7; previous revision: 1.99.2.6
done
Checking in Bugzilla/Token.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Token.pm,v  <--  Token.pm
new revision: 1.31.2.1; previous revision: 1.31
done
Checking in Bugzilla/DB/Mysql.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v  <--  Mysql.pm
new revision: 1.24.2.2; previous revision: 1.24.2.1
done
Checking in Bugzilla/DB/Pg.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Pg.pm,v  <--  Pg.pm
new revision: 1.12.2.3; previous revision: 1.12.2.2
done
Checking in contrib/sendunsentbugmail.pl;
/cvsroot/mozilla/webtools/bugzilla/contrib/sendunsentbugmail.pl,v  <--  sendunsentbugmail.pl
new revision: 1.5.4.1; previous revision: 1.5
done