-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
feat(database): Add replacements for deprecated fetch and fetchAll #40655
base: master
Are you sure you want to change the base?
feat(database): Add replacements for deprecated fetch and fetchAll #40655
Conversation
DBAL deprecated these and will remove them it in the future. Signed-off-by: Christoph Wurst <[email protected]>
…hAll Signed-off-by: Christoph Wurst <[email protected]>
* | ||
* @since 21.0.0 | ||
*/ | ||
public function fetchFirstColumn(): array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be mixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it returns the first column of all results as array<mixed>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description is not phrased clearly about that imo, and for better consistency perhaps consider renaming the method to fetchAllFirstColumns
or something like that?
lib/public/DB/IResult.php
Outdated
@@ -57,15 +57,37 @@ public function closeCursor(): bool; | |||
* @return mixed | |||
* | |||
* @since 21.0.0 | |||
* @deprecated 28.0.0 use fetchAssociative, fetchNumeric or fetchOne |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be painful :D
Touching all the queries yet again (after query() replacement)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also use the deprecation line from the docs here explaining the default replacement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChristophWurst Out of context, but I'm just curious to know if there are any plans to introduce Rector to the project. Now that I've touched a bit of code from different parts of the code base, I think it would be a great addition to the mix if we could make everything more consistent across the code base by using an automated refactoring approach with Rector.
…hAll Signed-off-by: Christoph Wurst <[email protected]>
…-result-fetch-associative-fetch-num
…hAll Signed-off-by: Christoph Wurst <[email protected]>
…hAll Signed-off-by: Christoph Wurst <[email protected]>
@@ -57,15 +57,37 @@ public function closeCursor(): bool; | |||
* @return mixed | |||
* | |||
* @since 21.0.0 | |||
* @deprecated 28.0.0 use fetchAssociative instead of fetch(), fetchNumeric instead of fetch(\PDO::FETCH_NUM) and fetchOne instead of fetch(\PDO::FETCH_COLUMN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @deprecated 28.0.0 use fetchAssociative instead of fetch(), fetchNumeric instead of fetch(\PDO::FETCH_NUM) and fetchOne instead of fetch(\PDO::FETCH_COLUMN) | |
* @deprecated 28.0.0 use fetchAssociative instead of fetch(), fetchNumeric() instead of fetch(\PDO::FETCH_NUM) and fetchOne() instead of fetch(\PDO::FETCH_COLUMN) |
🤪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and more brackets for lonely fetchAssociative
!
/** | ||
* @param int $fetchMode (one of PDO::FETCH_ASSOC, PDO::FETCH_NUM or PDO::FETCH_COLUMN (2, 3 or 7) | ||
* | ||
* @return mixed[] | ||
* | ||
* @since 21.0.0 | ||
* @deprecated 28.0.0 use fetchAllAssociative instead of fetchAll(), fetchAllNumeric instead of fetchAll(FETCH_NUM) and fetchOne instead of fetchAll(FETCH_COLUMN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @deprecated 28.0.0 use fetchAllAssociative instead of fetchAll(), fetchAllNumeric instead of fetchAll(FETCH_NUM) and fetchOne instead of fetchAll(FETCH_COLUMN) | |
* @deprecated 28.0.0 use fetchAllAssociative instead of fetchAll(), fetchAllNumeric() instead of fetchAll(FETCH_NUM) and fetchFirstColumn() instead of fetchAll(FETCH_COLUMN) |
🤪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm tired, boss
Summary
DBAL deprecated these and will remove them it in the future.
TODO
Checklist