php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #72759 Regression
Submitted: 2016-08-05 09:20 UTC Modified: 2016-08-15 11:41 UTC
From: yunosh@php.net Assigned: ab
Status: Closed Package: PDO PgSQL
PHP Version: master-Git-2016-08-05 (Git) OS:
Private report: No CVE-ID:
 [2016-08-05 09:20 UTC] yunosh@php.net
Description:
------------
Around ten days ago some change was implemented in the PDO_pgsql code that made some of our unit test starting to fail: https://travis-ci.org/horde/horde/jobs/148029445#L2160
Tests from July 26 succeeded, while they failed from July 28 on. Not sure how quick Travis is with updating the nightlies.
Interestingly, the Horde_SessionHandler tests are the only test with PDO_pgsql support that fail.
Yes, I know, you now want a small reproduceable test. But maybe we can further narrow down the possible changes in php-src so that I know where to start looking.


Patches

bug72759.patch (last revision 2016-08-10 21:55 UTC) by ab@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-08-05 09:25 UTC] yunosh@php.net
Looking at the history, this seems to be the only candidate that falls into the timespan: https://github.com/php/php-src/commit/12628e9a46b91a0aa92fd0619cdd545c409d25a6
 [2016-08-05 13:40 UTC] ab@php.net
-Status: Open +Status: Feedback
 [2016-08-05 13:40 UTC] ab@php.net
Thanks for the report. Were you already able to come up with a reproduce code?

Thanks.
 [2016-08-09 12:59 UTC] yunosh@php.net
Yes, I was able to trim this down today. It's about transactions. Commenting out the transaction statements in the following example makes the script work again:

<?php
$pdo = new PDO('pgsql:dbname=test', 'vagrant', '');
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
$pdo->query('DROP TABLE IF EXISTS "horde_sessionhandler"');
$pdo->query('CREATE TABLE "horde_sessionhandler" ( "session_id" character varying(32) NOT NULL, "session_lastmodified" integer NOT NULL, "session_data" bytea, PRIMARY KEY("session_id") )');
$pdo->query('CREATE INDEX "index_horde_sessionhandler_on_session_lastmodified" ON "horde_sessionhandler" ("session_lastmodified")');
$pdo->beginTransaction();
$pdo->query('INSERT INTO "horde_sessionhandler" ("session_id", "session_data", "session_lastmodified") VALUES (\'sessionid\', E\'\\\\x73657373696f6e64617461\', 1470745222)');
var_dump($pdo->lastInsertId(null));
$pdo->commit();
$stmt = $pdo->query('SELECT * FROM "horde_sessionhandler"');
var_dump($stmt);
var_dump($stmt->fetchAll(PDO::FETCH_ASSOC));
 [2016-08-10 21:00 UTC] ab@php.net
-Status: Feedback +Status: Analyzed
 [2016-08-10 21:00 UTC] ab@php.net
Thanks for the further research! Nope, it's not the transaction itself, but the changed behavior of lastinsertid() method. The call to $pdo->lastInsertId(null); implies a sequence were used. In this case, the table has no sequence column, so call to the last insert id is the error that ruins the transaction. It can be seen by viewing $stmt->errorinfo().

The PHP code seems to be not correct in first place, as lastinsertid() method should not be called without having a sequence. Therefore the bugfix is correct. But since it breaks the existing code, the question is probably only whether we revert this in the stable branch or some workaround is possible.

Thanks.
 [2016-08-10 21:55 UTC] ab@php.net
The following patch has been added/updated:

Patch Name: bug72759.patch
Revision:   1470866146
URL:        https://bugs.php.net/patch-display.php?bug=72759&patch=bug72759.patch&revision=1470866146
 [2016-08-14 18:07 UTC] ab@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=977cbc2fff1a3ec9d29a2c0904fae01bfd64c6c2
Log: Fixed bug #72759 Regression in pgo_pgsql
 [2016-08-14 18:07 UTC] ab@php.net
-Status: Analyzed +Status: Closed
 [2016-08-14 21:07 UTC] ab@php.net
-Assigned To: +Assigned To: ab
 [2016-08-14 21:07 UTC] ab@php.net
@yunosh, i've applied this patch to retain BC in stable branches. Though, it's unchanged in 7.1+, please be aware.

Thanks.
 [2016-08-15 11:41 UTC] yunosh@php.net
Thanks!
 [2016-10-17 10:09 UTC] bwoebi@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=977cbc2fff1a3ec9d29a2c0904fae01bfd64c6c2
Log: Fixed bug #72759 Regression in pgo_pgsql
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Fri Feb 24 01:01:37 2017 UTC