Skip to content

Commit add33e6

Browse files
jahnvi480jahnvi480
authored
FIX: Freeing metadata entries and clearing the vector (#1596)
* FIX: Freeing metadata entries and clearing the vector * Resolving comments
1 parent 32ecea7 commit add33e6

5 files changed

Lines changed: 300 additions & 28 deletions

File tree

‎source/pdo_sqlsrv/pdo_stmt.cpp‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,15 @@ int pdo_sqlsrv_stmt_execute( _Inout_ pdo_stmt_t *stmt )
588588

589589
SQLRETURN execReturn = core_sqlsrv_execute( driver_stmt, query, query_len );
590590

591+
// On re-execution, free stale PDO column descriptors and reset the
592+
// executed flag so that PDO re-describes columns for the new result
593+
// set (see GH#1466). Setting executed = 0 works around a PDO driver
594+
// manager optimization that otherwise skips the describe_col call.
595+
if ( stmt->columns ) {
596+
php_pdo_stmt_set_column_count( stmt, 0 );
597+
}
598+
stmt->executed = 0;
599+
591600
if ( execReturn == SQL_NO_DATA ) {
592601
stmt->column_count = 0;
593602
stmt->row_count = 0;
@@ -612,21 +621,6 @@ int pdo_sqlsrv_stmt_execute( _Inout_ pdo_stmt_t *stmt )
612621
stmt->row_count = driver_stmt->row_count;
613622
}
614623
}
615-
616-
// workaround for a bug in the PDO driver manager. It is fairly simple to crash the PDO driver manager with
617-
// the following sequence:
618-
// 1) Prepare and execute a statement (that has some results with it)
619-
// 2) call PDOStatement::nextRowset until there are no more results
620-
// 3) execute the statement again
621-
// 4) call PDOStatement::getColumnMeta
622-
// It crashes from what I can tell because there is no metadata because there was no call to
623-
// pdo_stmt_sqlsrv_describe_col and stmt->columns is NULL on the second call to
624-
// PDO::execute. My guess is that because stmt->executed is true, it optimizes away a necessary call to
625-
// pdo_sqlsrv_stmt_describe_col. By setting the stmt->executed flag to 0, this call is not optimized away
626-
// and the crash disappears.
627-
if( stmt->columns == NULL ) {
628-
stmt->executed = 0;
629-
}
630624
}
631625
catch( core::CoreException& /*e*/ ) {
632626

@@ -1225,11 +1219,9 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt )
12251219

12261220
core_sqlsrv_next_result( static_cast<sqlsrv_stmt*>( stmt->driver_data ) );
12271221

1228-
// clear the current meta data since the new result will generate new meta data
1229-
driver_stmt->clean_up_results_metadata();
1230-
1231-
// if there are no more result sets, return that it failed.
12321222
if( driver_stmt->past_next_result_end == true ) {
1223+
// Clean up remaining metadata since new_result_set() was not called
1224+
driver_stmt->clean_up_results_metadata();
12331225
return 0;
12341226
}
12351227

‎source/shared/core_stmt.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,9 @@ void sqlsrv_stmt::new_result_set( void )
206206
// delete sensivity data
207207
clean_up_sensitivity_metadata();
208208

209-
// reset sqlsrv php type in meta data
210-
size_t num_fields = this->current_meta_data.size();
211-
for (size_t f = 0; f < num_fields; f++) {
212-
this->current_meta_data[f]->reset_php_type();
213-
}
209+
// delete results metadata from the previous result set
210+
// to avoid stale metadata when re-executing a prepared statement
211+
clean_up_results_metadata();
214212

215213
// create a new result set
216214
if( cursor_type == SQLSRV_CURSOR_BUFFERED ) {

‎source/sqlsrv/stmt.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -571,11 +571,9 @@ PHP_FUNCTION( sqlsrv_next_result )
571571

572572
core_sqlsrv_next_result( stmt, true );
573573

574-
// clear the current meta data since the new result will generate new meta data
575-
stmt->clean_up_results_metadata();
576-
577574
if( stmt->past_next_result_end ) {
578-
575+
// Clean up remaining metadata since new_result_set() was not called
576+
stmt->clean_up_results_metadata();
579577
RETURN_NULL();
580578
}
581579

‎test/functional/pdo_sqlsrv/pdo_reexecute_multi_resultset.phpt‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
--TEST--
2+
GitHub issue 1466 - Re-executing prepared statement with multiple result sets
3+
--DESCRIPTION--
4+
Verifies that re-executing a prepared statement returning multiple result sets
5+
does not cause a fatal error or return corrupted data.
6+
--ENV--
7+
PHPT_EXEC=true
8+
--SKIPIF--
9+
<?php require('skipif_mid-refactor.inc'); ?>
10+
--FILE--
11+
<?php
12+
require_once("MsCommon_mid-refactor.inc");
13+
14+
try {
15+
$conn = connect();
16+
$conn->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
17+
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
18+
19+
$sql = "
20+
SET NOCOUNT ON;
21+
DECLARE @id INT = :id;
22+
-- Result set 1
23+
SELECT @id AS r1col1, 2 AS r1col2, 3 AS r1col3, N'Test' AS r1col4
24+
WHERE @id IN (2, 3);
25+
-- Result set 2
26+
SELECT @id AS r2col1, N'/test.txt' AS r2col2, N'text/plain' AS r2col3
27+
WHERE @id IN (2, 3);
28+
";
29+
30+
$stmt = $conn->prepare($sql);
31+
32+
$idList = [2, 1, 3];
33+
foreach ($idList as $id) {
34+
$stmt->bindValue(':id', $id);
35+
$stmt->execute();
36+
37+
// Fetch result set 1
38+
$rows1 = $stmt->fetchAll();
39+
foreach ($rows1 as $row) {
40+
if (!array_key_exists('r1col1', $row)) {
41+
throw new RuntimeException("Result set 1 row missing expected column 'r1col1'.");
42+
}
43+
if ($row['r1col1'] != $id) {
44+
throw new RuntimeException("Result set 1 r1col1 expected $id, got {$row['r1col1']}.");
45+
}
46+
}
47+
48+
// Move to result set 2
49+
$stmt->nextRowset();
50+
$rows2 = $stmt->fetchAll();
51+
foreach ($rows2 as $row) {
52+
if (!array_key_exists('r2col1', $row)) {
53+
throw new RuntimeException("Result set 2 row missing expected column 'r2col1'.");
54+
}
55+
if ($row['r2col1'] != $id) {
56+
throw new RuntimeException("Result set 2 r2col1 expected $id, got {$row['r2col1']}.");
57+
}
58+
}
59+
60+
$stmt->closeCursor();
61+
62+
// Verify row counts based on input
63+
if ($id == 1) {
64+
// id=1 is not in (2,3), so both result sets should be empty
65+
if (count($rows1) !== 0 || count($rows2) !== 0) {
66+
throw new RuntimeException("Expected empty result sets for id=1.");
67+
}
68+
} else {
69+
// id=2 or id=3 should return one row per result set
70+
if (count($rows1) !== 1 || count($rows2) !== 1) {
71+
throw new RuntimeException("Expected one row per result set for id=$id.");
72+
}
73+
}
74+
}
75+
echo "Test 1 passed: re-execute with closeCursor\n";
76+
77+
// Test 2: Re-execute without closeCursor (PDO auto-flushes remaining results)
78+
// This exercises the flush loop in pdo_sqlsrv_stmt_execute that calls
79+
// core_sqlsrv_next_result before the new execute.
80+
$stmt2 = $conn->prepare($sql);
81+
$stmt2->bindValue(':id', 2);
82+
$stmt2->execute();
83+
$row = $stmt2->fetch();
84+
// Do NOT call nextRowset or closeCursor -- just re-execute immediately
85+
$stmt2->bindValue(':id', 3);
86+
$stmt2->execute();
87+
$rows = $stmt2->fetchAll();
88+
if (count($rows) !== 1 || $rows[0]['r1col1'] != 3) {
89+
throw new RuntimeException("Test 2 failed: unexpected result after re-execute without closeCursor.");
90+
}
91+
$stmt2->closeCursor();
92+
echo "Test 2 passed: re-execute without closeCursor\n";
93+
94+
// Test 3: Re-execute when both result sets have the same number of columns
95+
// but different column names. Ensures stale column descriptors are refreshed
96+
// even when the column count hasn't changed.
97+
$sql3 = "
98+
SET NOCOUNT ON;
99+
DECLARE @v INT = :val;
100+
SELECT @v AS col_a, N'first' AS col_b;
101+
SELECT @v AS col_x, N'second' AS col_y;
102+
";
103+
$stmt3 = $conn->prepare($sql3);
104+
for ($i = 1; $i <= 3; $i++) {
105+
$stmt3->bindValue(':val', $i);
106+
$stmt3->execute();
107+
108+
$row1 = $stmt3->fetch();
109+
if (!array_key_exists('col_a', $row1) || !array_key_exists('col_b', $row1)) {
110+
throw new RuntimeException("Test 3 iteration $i: RS1 missing expected columns.");
111+
}
112+
if ($row1['col_a'] != $i) {
113+
throw new RuntimeException("Test 3 iteration $i: RS1 col_a expected $i, got {$row1['col_a']}.");
114+
}
115+
116+
$stmt3->nextRowset();
117+
$row2 = $stmt3->fetch();
118+
if (!array_key_exists('col_x', $row2) || !array_key_exists('col_y', $row2)) {
119+
throw new RuntimeException("Test 3 iteration $i: RS2 missing expected columns.");
120+
}
121+
if ($row2['col_x'] != $i) {
122+
throw new RuntimeException("Test 3 iteration $i: RS2 col_x expected $i, got {$row2['col_x']}.");
123+
}
124+
125+
$stmt3->closeCursor();
126+
}
127+
echo "Test 3 passed: same column count, different names\n";
128+
129+
echo "Done\n";
130+
} catch (Exception $e) {
131+
echo $e->getMessage() . "\n";
132+
}
133+
?>
134+
--EXPECT--
135+
Test 1 passed: re-execute with closeCursor
136+
Test 2 passed: re-execute without closeCursor
137+
Test 3 passed: same column count, different names
138+
Done

‎test/functional/sqlsrv/sqlsrv_reexecute_multi_resultset.phpt‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
--TEST--
2+
GitHub issue 1466 - Re-executing prepared statement with multiple result sets
3+
--DESCRIPTION--
4+
Verifies that re-executing a prepared statement returning multiple result sets
5+
does not cause a fatal error or return corrupted data.
6+
--SKIPIF--
7+
<?php require('skipif.inc'); ?>
8+
--FILE--
9+
<?php
10+
require_once('MsCommon.inc');
11+
12+
$conn = connect();
13+
if ($conn === false) {
14+
fatalError("Could not connect.\n");
15+
}
16+
17+
$sql = "
18+
SET NOCOUNT ON;
19+
DECLARE @id INT = ?;
20+
-- Result set 1
21+
SELECT @id AS r1col1, 2 AS r1col2, 3 AS r1col3, N'Test' AS r1col4
22+
WHERE @id IN (2, 3);
23+
-- Result set 2
24+
SELECT @id AS r2col1, N'/test.txt' AS r2col2, N'text/plain' AS r2col3
25+
WHERE @id IN (2, 3);
26+
";
27+
28+
$stmt = sqlsrv_prepare($conn, $sql, array(&$id));
29+
if ($stmt === false) {
30+
fatalError("sqlsrv_prepare failed.\n");
31+
}
32+
33+
$idList = array(2, 1, 3);
34+
foreach ($idList as $id) {
35+
if (!sqlsrv_execute($stmt)) {
36+
fatalError("sqlsrv_execute failed for id=$id.\n");
37+
}
38+
39+
// Fetch result set 1
40+
$rowCount1 = 0;
41+
while ($row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC)) {
42+
if (!array_key_exists('r1col1', $row)) {
43+
fatalError("Result set 1 row missing expected column 'r1col1'.\n");
44+
}
45+
if ($row['r1col1'] != $id) {
46+
fatalError("Result set 1 r1col1 expected $id, got {$row['r1col1']}.\n");
47+
}
48+
$rowCount1++;
49+
}
50+
51+
// Move to result set 2
52+
$nextResult = sqlsrv_next_result($stmt);
53+
if ($nextResult === false) {
54+
fatalError("sqlsrv_next_result failed for id=$id.\n");
55+
}
56+
57+
$rowCount2 = 0;
58+
if ($nextResult !== null) {
59+
while ($row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC)) {
60+
if (!array_key_exists('r2col1', $row)) {
61+
fatalError("Result set 2 row missing expected column 'r2col1'.\n");
62+
}
63+
if ($row['r2col1'] != $id) {
64+
fatalError("Result set 2 r2col1 expected $id, got {$row['r2col1']}.\n");
65+
}
66+
$rowCount2++;
67+
}
68+
}
69+
70+
// Verify row counts based on input
71+
if ($id == 1) {
72+
if ($rowCount1 !== 0 || $rowCount2 !== 0) {
73+
fatalError("Expected empty result sets for id=1.\n");
74+
}
75+
} else {
76+
if ($rowCount1 !== 1 || $rowCount2 !== 1) {
77+
fatalError("Expected one row per result set for id=$id.\n");
78+
}
79+
}
80+
}
81+
sqlsrv_free_stmt($stmt);
82+
echo "Test 1 passed: re-execute with next_result\n";
83+
84+
// Test 2: Re-execute without consuming all result sets (auto-flush)
85+
// This exercises the flush loop in sqlsrv_execute that calls
86+
// core_sqlsrv_next_result before the new execute.
87+
$id = 2;
88+
$stmt2 = sqlsrv_prepare($conn, $sql, array(&$id));
89+
if ($stmt2 === false) {
90+
fatalError("sqlsrv_prepare failed for test 2.\n");
91+
}
92+
sqlsrv_execute($stmt2);
93+
$row = sqlsrv_fetch_array($stmt2, SQLSRV_FETCH_ASSOC);
94+
// Do NOT call sqlsrv_next_result -- just re-execute immediately
95+
$id = 3;
96+
if (!sqlsrv_execute($stmt2)) {
97+
fatalError("sqlsrv_execute failed for test 2 re-execute.\n");
98+
}
99+
$row = sqlsrv_fetch_array($stmt2, SQLSRV_FETCH_ASSOC);
100+
if (!$row || !array_key_exists('r1col1', $row) || $row['r1col1'] != 3) {
101+
fatalError("Test 2 failed: unexpected result after re-execute without next_result.\n");
102+
}
103+
sqlsrv_free_stmt($stmt2);
104+
echo "Test 2 passed: re-execute without next_result\n";
105+
106+
// Test 3: Re-execute when both result sets have the same number of columns
107+
// but different column names.
108+
$sql3 = "
109+
SET NOCOUNT ON;
110+
DECLARE @v INT = ?;
111+
SELECT @v AS col_a, N'first' AS col_b;
112+
SELECT @v AS col_x, N'second' AS col_y;
113+
";
114+
$val = 0;
115+
$stmt3 = sqlsrv_prepare($conn, $sql3, array(&$val));
116+
if ($stmt3 === false) {
117+
fatalError("sqlsrv_prepare failed for test 3.\n");
118+
}
119+
for ($val = 1; $val <= 3; $val++) {
120+
if (!sqlsrv_execute($stmt3)) {
121+
fatalError("sqlsrv_execute failed for test 3, val=$val.\n");
122+
}
123+
$row1 = sqlsrv_fetch_array($stmt3, SQLSRV_FETCH_ASSOC);
124+
if (!$row1 || !array_key_exists('col_a', $row1) || $row1['col_a'] != $val) {
125+
fatalError("Test 3 val=$val: RS1 unexpected result.\n");
126+
}
127+
$next = sqlsrv_next_result($stmt3);
128+
if ($next === false) {
129+
fatalError("sqlsrv_next_result failed for test 3, val=$val.\n");
130+
}
131+
$row2 = sqlsrv_fetch_array($stmt3, SQLSRV_FETCH_ASSOC);
132+
if (!$row2 || !array_key_exists('col_x', $row2) || $row2['col_x'] != $val) {
133+
fatalError("Test 3 val=$val: RS2 unexpected result.\n");
134+
}
135+
}
136+
sqlsrv_free_stmt($stmt3);
137+
echo "Test 3 passed: same column count, different names\n";
138+
139+
sqlsrv_close($conn);
140+
echo "Done\n";
141+
?>
142+
--EXPECT--
143+
Test 1 passed: re-execute with next_result
144+
Test 2 passed: re-execute without next_result
145+
Test 3 passed: same column count, different names
146+
Done

0 commit comments

Comments
 (0)