-
Notifications
You must be signed in to change notification settings - Fork 122
Broader PDO fetch-style range #296
base: master
Are you sure you want to change the base?
Conversation
@chris-kruining thanks for this PR, I'm sorry to review it only now. The goal of your PR is fine (@Ocramius as library, we need to support all the PDO fetch modes), but I think it's better to check for valid PDO::FETCH_* constants using an array of allowed modes. use PDO;
class Result implements Iterator, ResultInterface
{
// ...
private $allowedFetchModes = [
PDO::FETCH_ASSOC,
PDO::FETCH_BOTH,
PDO::FETCH_BOUND,
PDO::FETCH_CLASS,
PDO::FETCH_CLASSTYPE,
PDO::FETCH_PROPS_LATE,
PDO::FETCH_INTO,
PDO::FETCH_LAZY,
PDO::FETCH_NAMED,
PDO::FETCH_NUM,
PDO::FETCH_OBJ
];
/**
* @param int $fetchMode
* @throws Exception\InvalidArgumentException on invalid fetch mode
*/
public function setFetchMode($fetchMode)
{
if (! in_array($fetchMode, $this->allowedFetchModes)) {
throw new Exception\InvalidArgumentException(
'The fetch mode must be one of the PDO::FETCH_* constants.'
);
}
$this->fetchMode = (int) $fetchMode;
}
} This change is required also for fix an issue on the previous implementation. In fact, checking for @chris-kruining can you change this PR according to my suggestion? Thanks a ton! |
@ezimuel I've made the changed like you proposed, however I do have some concerns: In my opinion is not a completely valid set as |
@ezimuel ping |
Would be great if this could be resolved guys, I think 1.5 years is a tad bit long :P |
@webimpress the issues you have mentioned should have been resolves afaik |
@chris-kruining Thanks, but now somehow I see conflicts on this PR. Can you resolve them, please? |
this however in my opinion is not a completely valid set as `FETCH_PROPS_LATE` and `FETCH_CLASSTYPE` should only be used in combination with `FETCH_CLASS`
@webimpress fixed |
ping |
This repository has been moved to laminas/laminas-db. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:
|
This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at laminas/laminas-db#59. |
I encountered a situation Where I want to fetch multiple columns with the same name where aliases are not an option, so I set the fetchmode to
\PDO::FETCH_NAMED
which resulted in theException\InvalidArgumentException
"The fetch mode must be one of the PDO::FETCH_* constants.", which\PDO::FETCH_NAMED
is.I've attempted to fix this by editing the condition so that all the fetch styles listed in http://php.net/manual/en/pdostatement.fetch.php
I hope that this PR is to the required standards :D