Cursor is not opened on singleton selects. #2241

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@madorin
Contributor
madorin commented Dec 15, 2016

In case if we execute twice a prepared singleton statements an error is thrown trying to close an unopened cursor.

Code sample:
$S = $D->prepare('insert into ta_test (id, name) values (next value for gs_id, :name) returning id');
$S->execute(['name' => 'One']);
$S->execute(['name' => 'Two']);

All current tests are passed with this commit change.

@staabm
Contributor
staabm commented Dec 15, 2016

you should add a new test which fails without your patch and succeeds with it

@madorin
Contributor
madorin commented Dec 15, 2016

Don't know if it is worth to include it in vcs.

Here is the test code:

--TEST--
PDO_Firebird: cursor should not be marked as opened on singleton statements
--SKIPIF--
<?php if (!extension_loaded('interbase') || !extension_loaded('pdo_firebird')) die('skip'); ?>
--FILE--
<?php
require 'testdb.inc';
$C = new PDO('firebird:dbname='.$test_base, $user, $password, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]) or die;
@$C->exec('drop table ta_table');
$C->exec('create table ta_table (id integer, name varchar(20))');
$S = $C->prepare('insert into ta_table (id, name) values (:id, :name) returning name');
$S->execute(['id' => 1, 'name' => 'one']);
$S->execute(['id' => 2, 'name' => 'two']);
$D = $S->fetchAll();
$S->execute(['id' => 3]);
unset($S);
unset($C);
echo 'OK';
?>
--EXPECT--
OK

An failed results on current master branch:

d:\Projects\Php\phpdev\vc14\x64>x64\Debug_TS\php.exe run-tests.php -v --show-all ext\pdo_firebird\tests\bug_aa.phpt

=====================================================================
PHP         : x64\Debug_TS\php.exe
PHP_SAPI    : cli
PHP_VERSION : 7.2.0-dev
ZEND_VERSION: 3.2.0-dev
PHP_OS      : WINNT - Windows NT DESKTOP-GJMTUI6 10.0 build 14393 (Windows 10) AMD64
INI actual  : D:\Projects\Php\phpdev\vc14\x64
More .INIs  :
CWD         : D:\Projects\Php\phpdev\vc14\x64
Extra dirs  :
VALGRIND    : Not used
=====================================================================
Running selected tests.

=================
TEST D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.phpt
TEST 1/1 [D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.phpt]
========SKIP========
<?php if (!extension_loaded('interbase') || !extension_loaded('pdo_firebird')) die('skip'); ?>
========DONE========

========TEST========
<?php
require 'testdb.inc';
$C = new PDO('firebird:dbname='.$test_base, $user, $password, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]) or die;
@$C->exec('drop table ta_table');
$C->exec('create table ta_table (id integer, name varchar(20))');
$S = $C->prepare('insert into ta_table (id, name) values (:id, :name) returning name');
$S->execute(['id' => 1, 'name' => 'one']);
$S->execute(['id' => 2, 'name' => 'two']);
$D = $S->fetchAll();
$S->execute(['id' => 3]);
unset($S);
unset($C);
echo 'OK';
?>
========DONE========

CONTENT_LENGTH  =
CONTENT_TYPE    =
PATH_TRANSLATED = D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php
QUERY_STRING    =
REDIRECT_STATUS = 1
REQUEST_METHOD  = GET
SCRIPT_FILENAME = D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php
HTTP_COOKIE     =
COMMAND x64\Debug_TS\php.exe   -d "output_handler=" -d "open_basedir=" -d "disable_functions=" -d "output_buffering=Off" -d "error_reporting=32767" -d "display_errors=1" -d "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d "track_errors=1" -d "report_memleaks=1" -d "report_zend_debug=0" -d "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" -d "ignore_repeated_errors=0" -d "precision=14" -d "memory_limit=128M" -d "log_errors_max_len=0" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -f "D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php"  2>&1

========OUT========
Fatal error: Uncaught PDOException: SQLSTATE[HY000]: General error: -501 Attempt to reclose a closed cursor  in D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php:11
Stack trace:
#0 D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php(11): PDOStatement->execute(Array)
#1 {main}
  thrown in D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php on line 11
========DONE========

========EXP========
OK
========DONE========

========DIFF========
001+ Fatal error: Uncaught PDOException: SQLSTATE[HY000]: General error: -501 Attempt to reclose a closed cursor  in D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php:11
001- OK
002+ Stack trace:
003+ #0 D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php(11): PDOStatement->execute(Array)
004+ #1 {main}
005+   thrown in D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.php on line 11
========DONE========
FAIL PDO_Firebird: cursor should not be marked as opened on singleton statements [D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.phpt]
=====================================================================
Number of tests :    1                 1
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    1 (100.0%) (100.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :    0 (  0.0%) (  0.0%)
---------------------------------------------------------------------
Time taken      :    0 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
PDO_Firebird: cursor should not be marked as opened on singleton statements [D:\Projects\Php\phpdev\vc14\x64\ext\pdo_firebird\tests\bug_aa.phpt]
=====================================================================

d:\Projects\Php\phpdev\vc14\x64>
@weltling
Contributor

@madorin thanks for the PR, Dorin. Every change usually requires an accompanying test, as this way we a) ensure the regression is fixed and b) ensure that there is no breach in the future. Please add this test case to the PR code, and in general supply tests also in the future. The tests to pdo_firebird are unfortunately not included in the travis runs, so the more important to have a good test coverage for this ext.

Thanks.

@madorin
Contributor
madorin commented Dec 15, 2016 edited

Hi Anatol! Added test case.
Hope to fix all failing current pdo_firebird related tests in near future for later inclusion in Travis.
p.s. This test fails on Firebird3.0.

@weltling
Contributor

@madorin, nice! I'll be testing the PR next days, if no one beats me to it.

Thanks.

@madorin madorin Set error mode to warning instead of exception.
e33590b
@weltling
Contributor

Merged as cf46ac1.

Thanks.

@weltling weltling closed this Dec 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment