Fix #69592: pdo_dblib option to skip empty rowsets #2846
Conversation
This adds a new attribute PDO::DBLIB_ATTR_SKIP_EMPTY_ROWSETS to enable automatic skipping of empty rowsets. This happens with some sql statements (like print or set): a rowset with 0 columns is returned by the driver. With this option enabled 0 columns rowsets are automatically skipped, mirroring the behavior of the former mssql driver. Credits go to MiRacLe-RPZ for developping and promoting this patch.
|
cc'ing maintainer @adambaratz |
Bugfix: revert erroneous delete...
Add as a __construct driver option
ext/pdo_dblib/dblib_stmt.c
Outdated
| } while (H->skip_empty_rowsets && num_fields <= 0 && ret == SUCCEED); | ||
|
|
||
|
|
||
| if (H->skip_empty_rowsets && num_fields <= 0) { |
There was a problem hiding this comment.
I think it would be better to move this check after the one for FAIL == ret. There might be a situation where you could wind up suppressing a desired error message. The return values would otherwise be the same.
There was a problem hiding this comment.
I guess it could happen (in case of en empty rowset with an error), I just moved the check after the two others.
From my understanding this new check is a failsafe, in case of an empty rowset and a "ret" value different from FAIL and NO_MORE_RESULTS, so it might as well be the last one...
|
Aside from the one comment I left, looks fine. Could you confirm that the proposed change won't have any undesirable side effects? |
Fix error handling in next_rowset
|
As far as I know, with the option off the behavior is exactly the same as before, so aside from a bug there should not be any undesirable effect. Also in my opinion skipping those empty rowsets is the safest practice: that was the former mssql driver behavior and it makes our code resilient to a change in SQL (if a junk-rowset-producing statement is added to the query the consuming code doesn't need to be updated). Moreover from what I gather, since a recent patch (this one, distributed since 1.00.23) FreeTDS can now produce empty rowsets in between regular ones (whereas before it happened only before the first real one) and so code relying on the default behavior might have to be updated. My 2 cents is that most code actualy dealing with multiple rowsets either implements this fix in php (as you suggested in the bug report) or work around it in SQL. |
|
Comment on behalf of adambaratz at php.net: merged into master |
This adds a new attribute PDO::DBLIB_ATTR_SKIP_EMPTY_ROWSETS to enable automatic skipping of empty rowsets.
This happens with some sql statements (like print or set): a rowset with 0 columns is returned by the driver.
With this option enabled 0 columns rowsets are automatically skipped, mirroring the behavior of the former mssql driver.
Credits go to MiRacLe-RPZ for developping and promoting this patch.
The PR is an attempt to resurect #1758 by the original author