Introduce fetch* methods in query builder

[andrew-demb]

Q	A
Type	feature
BC Break	no
Fixed issues	#4461

Summary

Added new fetch* methods (all non-deprecated fetch* methods from Connection class) to allow the use of better typing in fetching data.

Please let me know if I should write some tests and what to use in the description of the methods.

[morozov]

Please let me know if I should write some tests

Yes, please do. It could be a set of unit tests that all use the same QueryBuilder instance producing a couple of rows and a couple of columns and testing all the new methods. Something similar to ConverterTest.

[...] what to use in the description of the methods.

You can copy-paste the description of the corresponding Statement/Result methods or leave them empty.

[andrew-demb]

It could be a set of unit tests that all use the same QueryBuilder instance producing a couple of rows and a couple of columns and testing all the new methods. Something similar to ConverterTest.

To be in sync - are you suggest a "fill" query builder with some data and use the mock connection with a strictly expected query to be bypassed to the needed method?

Example (expected query and other will be parametrized from data provider):

    protected function setUp(): void
    {
        $this->conn = $this->createMock(Connection::class);
    }

    public function testFetchAssociative(): void
    {
        $qb = new QueryBuilder($this->conn);

        $this->conn->expects($this->once())
            ->method('fetchAssociative')
            ->with('SELECT some FROM table');

        $qb->select('some')
            ->from('table')
            ->fetchAssociative();
    }

Thank you for help.

[morozov]

Yeah, the above looks good. You can also add the assertions that not only the query but the parameters and types are passed downstream, and the value returned by the statement is returned. This is as much as these new methods do.

[morozov]

As for the target branch, there's no plan to tag any new 2.x releases potentially with the exception of improving the upgrade path to 3.0. This improvement doesn't fall into the "upgrade path" category, so I believe it should target 3.1.x.

[andrew-demb]

Tests added.
Ready for review

[morozov]

Looks good, @andrew-demb. Please squash, and I can merge it. Or I can squash it before the merge.

[PowerKiKi]

I think this PR lacks the addition of a new method executeStatement(), to be really complete. It could be done in a second PR, but I feel it would be more coherent to add all methods together, don't you think ?

[morozov]

I think this PR lacks the addition of a new method executeStatement(), to be really complete. It could be done in a second PR, but I feel it would be more coherent to add all methods together, don't you think ?

This PR doesn't deprecate anything so it's complete by just adding new methods. If you want to work on the deprecation, in addition to executeStatement() we'll need to add executeQuery() as well. Otherwise, there will be no way to obtain a Result from the built query.

[andrew-demb]

@morozov thank you for squash

@PowerKiKi sorry, I missed your messages before. I think a new deprecation and more methods may require an additional discussion.
Lets do it in additional PR.