FIX: Freeing metadata entries and clearing the vector

[jahnvi480]

Github Issue #1466
This pull request addresses a critical bug (GH#1466) where re-executing a prepared statement that returns multiple result sets with different column layouts could cause crashes or data corruption in the SQLSRV and PDO_SQLSRV drivers. The changes ensure that stale column metadata is properly cleared between executions, and comprehensive tests are added to verify correct behavior in various scenarios.

Bug Fixes:

• Ensured that stale PDO column descriptors are freed before re-executing a prepared statement in pdo_stmt.cpp, so that columns are correctly re-described for new result sets and to prevent crashes or data corruption.
• Updated core_stmt.cpp to delete results metadata from the previous result set when starting a new result set, preventing stale metadata issues on re-execution.

Testing Improvements:

• Added a functional test pdo_reexecute_multi_resultset.phpt for PDO_SQLSRV that verifies re-executing prepared statements with multiple result sets does not cause fatal errors or data corruption, including edge cases with and without cursor closing and with different column names.
• Added a similar functional test sqlsrv_reexecute_multi_resultset.phpt for SQLSRV, ensuring consistent behavior and validating the fix for both drivers.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.75%. Comparing base (d1027a4) to head (661cae8).

Additional details and impacted files

@@           Coverage Diff           @@
##              dev    #1596   +/-   ##
=======================================
  Coverage   85.75%   85.75%           
=======================================
  Files          23       23           
  Lines        7211     7211           
=======================================
  Hits         6184     6184           
  Misses       1027     1027           

Files with missing lines	Coverage Δ
source/pdo_sqlsrv/pdo_stmt.cpp	81.75% <100.00%> (+0.06%)	⬆️
source/shared/core_stmt.cpp	93.49% <100.00%> (-0.01%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[David-Engel]

Here are my recommendations, ordered by importance:

---

1. Keep the new_result_set() change — it's the correct fix

Replacing reset_php_type() with clean_up_results_metadata() in new_result_set() is right. The old code only reset the cached PHP type mapping but preserved stale SQL metadata (column names, types, sizes). That's the root cause of the bug. No change needed here.

---

2. Scope the explicit cleanup in nextRowset/next_result to only the end-of-results case

core_sqlsrv_next_result() calls new_result_set() when there are more results, which now does the cleanup. It does not call new_result_set() when past_next_result_end — so the explicit call is only needed there. Currently it runs unconditionally, causing a redundant double-cleanup.

In stmt.cpp, change to:

core_sqlsrv_next_result( stmt, true );

if( stmt->past_next_result_end ) {
    // Clean up remaining metadata since new_result_set() was not called
    stmt->clean_up_results_metadata();
    RETURN_NULL();
}

RETURN_TRUE;

In pdo_stmt.cpp, change to:

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

if( driver_stmt->past_next_result_end == true ) {
    driver_stmt->clean_up_results_metadata();
    return 0;
}

stmt->column_count = core::SQLNumResultCols( driver_stmt );
// ... rest unchanged

This eliminates the redundant path and makes the cleanup responsibility clear: new_result_set() owns cleanup when advancing to a valid result set; explicit cleanup happens only at end-of-results.

---

3. Simplify the PDO executed workaround block

After php_pdo_stmt_set_column_count(stmt, 0), stmt->columns is always NULL. So the old workaround guard if (stmt->columns == NULL) always evaluates to true. Merge the two into a single coherent block:

// On re-execution, free stale PDO column descriptors and reset the
// executed flag so that PDO re-describes columns for the new result
// set (see GH#1466). Setting executed = 0 works around a PDO driver
// manager optimization that otherwise skips the describe_col call.
if ( stmt->columns ) {
    php_pdo_stmt_set_column_count( stmt, 0 );
}
stmt->executed = 0;

Remove the old separated workaround block and its lengthy comment. The executed = 0 is now always needed on re-execution (not just the niche nextRowset → execute → getColumnMeta sequence), and the new comment explains both mechanisms together. On first execution, columns is NULL so we skip the free, but executed = 0 is harmless (it's already 0).

---

Summary

Recommendation	Why
Keep clean_up_results_metadata() in new_result_set()	Core correctness fix
Scope explicit cleanup to past_next_result_end only	Eliminates redundant double-cleanup, clarifies ownership
Merge php_pdo_stmt_set_column_count + executed = 0	Removes dead code path, makes intent explicit