Skip to content

Fix #69592: pdo_dblib now skips datasets without columns while fetching data - #1758

Closed
MiRacLe-RPZ wants to merge 3 commits into
php:PHP-5.6from
MiRacLe-RPZ:bug-69592
Closed

MiRacLe-RPZ wants to merge 3 commits into
php:PHP-5.6from
MiRacLe-RPZ:bug-69592

Conversation

@MiRacLe-RPZ

Copy link
Copy Markdown
Contributor

Some sql-statements can return empty resultsets, now pdo_dblib skips empty datasets like ext/mssql.

Some sql-statements can return empty resultsets,
now pdo_dblib skips empty datasets like ext/mssql.
@adambaratz

Copy link
Copy Markdown
Contributor

I like this change, but there's probably code in the wild that relies on this (admittedly broken) behavior. It would make for smoother PHP upgrades if this was an opt-in behavior, via a driver attribute or whatever.

@MiRacLe-RPZ

Copy link
Copy Markdown
Contributor Author

@adambaratz, several versions ago "code in the wild" cannot be relies on "that", because call of nextResult causes segfault (see PR #1386). This behaviour copied from ext/mssql(dblib).

@adambaratz

Copy link
Copy Markdown
Contributor

I haven't dug into the code enough to understand why, but the segfaults weren't a consistent issue. My company has a fair bit of code that expects empty rowsets.

Would it be hard to add a boolean attribute to control this? It could be deprecated after a few minor revisions, but it would make it possible to transition out of this behavior instead of causing code to break when you upgrade PHP.

@MiRacLe-RPZ

Copy link
Copy Markdown
Contributor Author

@adambaratz can you show testcase with code that expects empty rowset(s) ?

P.S. - a "empty rowset" i mean dataset without columns, not rows...

@MiRacLe-RPZ MiRacLe-RPZ changed the title Fix #69592: pdo_dblib now skips empty resultsets while fetching data Fix #69592: pdo_dblib now skips datasets without columns while fetching data Feb 26, 2016
@adambaratz

Copy link
Copy Markdown
Contributor

The gist would be to change your example like this:

$stmt = $db->query($sql);
$stmt->fetchAll(); // swallow empty rowset...
$stmt->nextRowset(); // ...and move to the next
var_dump($stmt->fetchAll(PDO::FETCH_ASSOC)); // expected: array(result => 0), actual: array()
var_dump($stmt->nextRowset()); // expected: bool(false), actual: bool(true)
var_dump($stmt->fetchAll(PDO::FETCH_ASSOC)); // expected: array(), actual: array(result => 0)

@MiRacLe-RPZ

Copy link
Copy Markdown
Contributor Author

@adambaratz ok, i will try to add this functional optional by default, last question (programmer's main problem) - name for this pdo-attribute ?

lesilent pushed a commit to lesilent/Yau that referenced this pull request Feb 26, 2016
@adambaratz

Copy link
Copy Markdown
Contributor

It seems like the convention for driver-specific attrs is PDO::[DRIVER]ATTR[name]. I think PDO::DBLIB_ATTR_SKIP_EMPTY_ROWSETS would be fine.

@weltling

weltling commented Apr 6, 2016

Copy link
Copy Markdown
Contributor

Is there a 7.0 compatible patch for this issue?

Thanks.

@smalyshev smalyshev added the Bug label Nov 27, 2016
@krakjoe

krakjoe commented Jan 3, 2017

Copy link
Copy Markdown
Member

Since this targets a security fix only branch, and since the author seems to have abandoned working on it (or responding to questions/comments), I'm closing this PR.

Please take this action as (more) encouragement to open a PR against a supported branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants