Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Conversation

MiRacLe-RPZ
Copy link
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
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
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
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
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
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
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
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
Copy link
Contributor

weltling commented Apr 6, 2016

Is there a 7.0 compatible patch for this issue?

Thanks.

@smalyshev smalyshev added the Bug label Nov 27, 2016
@krakjoe
Copy link
Member

krakjoe commented Jan 3, 2017

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