Support stored procedures (multiple result sets)

[drucifer]

Calling a stored procedure results in an error:

React\MySQL\Exception: PROCEDURE test.simple can't return a result set in the given context in [path to my project]/react/mysql/src/Io/Parser.php on line 231

To reproduce, define a simple MySQL stored procedure:

mysql> delimiter //
USE test
CREATE PROCEDURE simple()
BEGIN
 SELECT CURRENT_DATE;
END;
//

call test.simple()//
+--------------+
| CURRENT_DATE |
+--------------+
| 2023-02-17   |
+--------------+

In PHP:

$dbh->query('CALL test.simple()', [])
  ->then(function (QueryResult $res) {
      var_dump([$res]);
    }, function ($ex) {
      var_dump($ex);
    });

[clue]

@drucifer Thanks for reporting, I can confirm that calling stored procedures using the CALL query is not currently supported by this project :+1:

The reason for this is that stored procedures may contain multiple statements and as such may return multiple results and our query API is not really suited for exposing multiple possible results. In other words, what kind of data structure would you expect the following stored procedure to return?

DELIMITER //
CREATE PROCEDURE not_so_simple()
BEGIN
  SELECT …;
  UPDATE …;
  SELECT …;
END;
//

In this case, we may want to expose an array(?) of QueryResult objects, but see also discussion in #151 if complicating our APIs is really worth it.

In your original example, your stored procedure only returns a single result set, so we would be able to expose this using our existing APIs, but the question would remain what would be supposed to happen if the stored procedure would return multiple result sets.

From a technical perspective, this boils down to specifying the CLIENT_MULTI_RESULTS constant in the AuthenticateCommand class. You can patch this manually and should be able to see that your simple example works just fine, but our current parser and APIs would trip over any stored procedure that does not return exactly one result set.

I suppose we're happy to accept PRs that implement support for this or if you need this for a commercial project and you want to help sponsor this feature, feel free to reach out and I'm happy to take a look. Other than that, there are currently no immediate plans to build this from my end (no demand at the moment and more important outstanding issues currently).

Let us know if you're interested in sponsoring this or want to work on this yourself 👍

[drucifer]

I suspected that was the reasoning, but wanted to confirm that this was an intentional omission and not just an oversight, as I did not see it mentioned or documented. It is not a high priority for us at the moment - our current PHP applications do not make heavy use of stored procedures, as the legacy mysql_query() had a similar limitation. However, we had been hoping to move in that direction as we uplift some of our older codebases, as many other applications in our company use stored procedures almost exclusively.

From an API perspective, I think the cleanest way would be to make QueryResult into a linked list with a '->nextResult' pointer to another QueryResult. This avoids creating a function that might return one object or might return an array of objects depending on how it is called. It would still be compatible with the existing API, and essentially transparent to anyone that isn't specifically expecting multiple results.

If that is an acceptable API change for you, I can look into creating a pull request in the future if we decide that this is important for us. I can ask about sponsorship as well. We have done it in the past for other open source projects.

[clue]

Definitely interested in support for stored procedures for this project! I like your idea of using a linked list of QueryResult objects, but I would be curious how this turns out with regards to buffering large result sets and also our streaming APIs. Let me know if there's anything we can do to make this happen :+1:

[devblack]

Using the CLIENT_MULTI_RESULTS gives the error Fatal error: Uncaught AssertionError: assert($command instanceof QueryCommand) in mysql\src\Io\Parser.php:370. This when calling a CALL simple_select(); followed by a SELECT * FROM settings;.

To reproduce (examples/01-query.php):

    $userInfo = await($connection->query('CALL simple_select();'));
    print_r($userInfo->resultRows);

    $settings = await($connection->query('SELECT * FROM settings;'));
    print_r($settings->resultRows);

Error log:

Fatal error: Uncaught AssertionError: assert($command instanceof QueryCommand) in Q:\mysql\src\Io\Parser.php:370
Stack trace:
#0 Q:\mysql\src\Io\Parser.php(370): assert(false, 'assert($command...')
#1 Q:\mysql\src\Io\Parser.php(291): React\MySQL\Io\Parser->onResultDone()
#2 Q:\mysql\src\Io\Parser.php(192): React\MySQL\Io\Parser->parsePacket(Object(React\MySQL\Io\Buffer))
#3 Q:\mysql\vendor\evenement\evenement\src\Evenement\EventEmitterTrait.php(123): React\MySQL\Io\Parser->handleData('.......')
#4 Q:\mysql\vendor\react\stream\src\Util.php(71): Evenement\EventEmitter->emit('data', Array)
#5 Q:\mysql\vendor\evenement\evenement\src\Evenement\EventEmitterTrait.php(123): React\Stream\Util::React\Stream\{closure}('.......')
#6 Q:\mysql\vendor\react\stream\src\DuplexResourceStream.php(196): Evenement\EventEmitter->emit('data', Array)
#7 Q:\mysql\vendor\react\event-loop\src\StreamSelectLoop.php(246): React\Stream\DuplexResourceStream->handleData(Resource id #66)
#8 Q:\mysql\vendor\react\event-loop\src\StreamSelectLoop.php(213): React\EventLoop\StreamSelectLoop->waitForStreamActivity(NULL)
#9 Q:\mysql\vendor\react\event-loop\src\Loop.php(211): React\EventLoop\StreamSelectLoop->run()
#10 Q:\mysql\vendor\react\async\src\SimpleFiber.php(57): React\EventLoop\Loop::run()
#11 [internal function]: React\Async\SimpleFiber::React\Async\{closure}()
#12 Q:\mysql\vendor\react\async\src\SimpleFiber.php(61): Fiber->resume()
#13 [internal function]: React\Async\SimpleFiber::React\Async\{closure}()
#14 {main}
  thrown in Q:\mysql\src\Io\Parser.php on line 370

Process finished with exit code 255

[SimonFrings]

@devblack You see this output, because it's currently not supported by this project. I think this is the behavior @clue described above:

From a technical perspective, this boils down to specifying the CLIENT_MULTI_RESULTS constant in the AuthenticateCommand class. You can patch this manually and should be able to see that your simple example works just fine, but our current parser and APIs would trip over any stored procedure that does not return exactly one result set.

Just patching the source code without the feature itself supported will end up in an error, so setting this flag currently doesn't make much sense. I think once we support calling stored procedures using the CALL query, we might also set this flag by default.