Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

Fix SQLite scroll cursors fail #36

Merged
merged 2 commits into from
Apr 13, 2016
Merged

Fix SQLite scroll cursors fail #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09f2b2f
Add a missing dependency.
Metalaka Apr 6, 2016
8153868
Don't use cursors when they aren't supported by the driver.
Metalaka Apr 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Layer/Pdo/Pdo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ protected function setConnection(\PDO $connection)
/**
* Get the connection instance.
*
* @return PDO
* @return \PDO
* @throws \Hoa\Database\Exception
*/
protected function getConnection()
Expand Down Expand Up @@ -194,7 +194,12 @@ public function lastInsertId($name = null)
public function prepare($statement, array $options = [])
{
if (!isset($options[\PDO::ATTR_CURSOR])) {
$options[\PDO::ATTR_CURSOR] = \PDO::CURSOR_SCROLL;
try {
$this->getConnection()->getAttribute(\PDO::ATTR_CURSOR);
$options[\PDO::ATTR_CURSOR] = \PDO::CURSOR_SCROLL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the ATTR_CURSOR is supported, but not the CURSOR_SCROLL value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try with this branch ?

I had the same error as you and this patch fix it (with sqlite3 on win10 x64)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I will try. However, iterator support will no be longer be available if scroll cursor is not enabled. Should we throw an error then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if this patch works, then this is a bug in PDO with SQLite driver and we should open a bug on bugs.php.net, do you agree?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if this patch works, then this is a bug in PDO with SQLite driver and we should open a bug on bugs.php.net, do you agree?

Yes.

Hmm, I will try. However, iterator support will no be longer be available if scroll cursor is not enabled. Should we throw an error then?

Forward only iterator will be available. Scrollable cursor is used to customize the fetch orientation.

Throw an error if the user want to use a feature which is not supported by the driver is interesting but we have to re-think the way we prepare our statement.
Currently we enable the scroll cursor before the user has chosen if he want to use the feature so we can't throw an error according to his choice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with your strategy. Are you willing to do a PR?

} catch (\PDOException $e) {
// Cursors isn't supported by the driver, see issue #35.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

- isn't
+ are not

Fixed by me.

}
}

$handle = $this->getConnection()->prepare($statement, $options);
Expand Down
1 change: 1 addition & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"hoa/event" : "~1.0",
"hoa/exception" : "~1.0",
"hoa/iterator" : "~2.0",
"hoa/protocol" : "~1.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

"hoa/zformat" : "~1.0",
"ext-pdo" : "*"
},
Expand Down