Skip to content

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