Improve IResult

[CarlSchwan]

Best to review commit by commit. The last two are automated

Summary

Expose multiple methods from result from doctrine:

• fetchAssociative
• fetchNumeric
• fetchOne
• fetchAllAssociate
• fetchAllNumeric
• fetchFirstColumn
• iterateNumeric
• iterateAssociate

Deprecated

• fetch and fetchAll

A rector rule is also created to automate the transition nextcloud-libraries/rector#63

TODO

• ...

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

[provokateurin]

I don't get the purpose of iterateNumeric and iterateAssociative if they just call the fetch methods. Developers will probably think that this optimizes memory consumption by not loading all results at once, but this is not the case as far as I understand from this code.

[ChristophWurst]

Very nice

This will impact a lot of app code, so thanks for the rector rule 🙏

There should also be documentation for app devs including examples given the impact.

[nickvergessen]

Deprecated

• fetch and fetchAll

Is this really needed? This, same as with execute(), will mean that every (select) query has to be touched again or you add hundreds of lines into your baseline or run rector which breaks backports.

I'd be in favor of making a note, and once we have few things we want to deprecate, we do them in one organized released, instead of each release bringing a single change and you need to touch each place all the time

[miaulalala]

Why deprecate fetch / fetchAll? Instead, leave that with the default associative and remove the fetch mode parameter. Leave those code changes where no other result type is used, and refactor those instances where a PDO::FETCH_NUMERIC is used to the new exposed method.

Benefits: no Psalm errors, no backport conflicts, marginal change in the interface

[CarlSchwan]

Why deprecate fetch / fetchAll? Instead, leave that with the default associative and remove the fetch mode parameter. Leave those code changes where no other result type is used, and refactor those instances where a PDO::FETCH_NUMERIC is used to the new exposed method.

Benefits: no Psalm errors, no backport conflicts, marginal change in the interface

I undeprecated fetch and fetchAll for now but we can't change the signature and behavior of these methods, otherwise it breaks a lot of stuff and we should avoid breaking changes.

[CarlSchwan]

Documentation nextcloud/documentation#13866

upvoted my comment

[blizzz]

What also would be cool is a Result interface that does not take the complete result array, but the pointer to the DB result resource. And then it can truly traverse memory-efficiently through the result set. Could be beneficial for network saturation in some cases as well, maybe?

(Out of scope here)