PDOStatementProxy->setFetchMode() to match that of PDOStatement

[yespire]

about this PR

• currently calling setFetchModel(PDO::FETCH_COLUMN, 0) would leads to the following exception:

PHP Fatal error: Uncaught ArgumentCountError: PDOStatement::setFetchMode() expects exactly 2 arguments for the fetch mode provided, 3 given in @swoole-src/library/core/Database/PDOStatementProxy.php:132

• PDOStatement support 3 modes expecting different parameters
  • https://www.php.net/manual/en/pdostatement.setfetchmode.php

for anyone encounters the same error message, alternative:

• skip setFetchMode() call
• use $stmt->fetchColumn(colno)

[sy-records]

You need to execute composer cs-fix

[yespire]

You need to execute composer cs-fix

command is showing error:

> /usr/bin/env php -d swoole.enable_library=Off ./vendor/bin/php-cs-fixer fix
Could not open input file: ./vendor/bin/php-cs-fixer
Script /usr/bin/env php -d swoole.enable_library=Off ./vendor/bin/php-cs-fixer fix handling the cs-fix event returned with error code 1

maybe you can hint me styling in which line(s) are not conforming, I can do a manually update

[yespire]

You need to execute composer cs-fix

Here is my guess:

public function setFetchMode(int $mode, $colno_class_object = null, array $ctorarfg = [])

probably because 2nd parameter can not be type hinted, if that's case, maybe we have to setup 3+ setup methods to cover the underlying PDOStatement->setFetchMode()

PDOStatementProxy

• setFetchMode()
• setFetchModeByColumn()
• setFetchModeByClass()
• setFetchModeByObject()

[twose]

How about ...$args ? If it works, code can be simplify and we just lose the paramaters tip (we can also add comment on this function).

[yespire]

How about ...$args ? If it works, code can be simplify and we just lose the paramaters tip (we can also add comment on this function).

refactored

[yespire]

Code check is complaining about N-path / Cyclomatic complexity level

i see the if condition / logic branching to be necessary for the method.

However we can refactor the if-block into a switch or a function mapping, but it is a bit subjective if this is more readable than if-block.

[twose]

1. Add ignore files to your own user ignore list is better, or do this in another PR.
2. I am so sorry that I recongnized that my idea is bad for PHP-8.x, it will break support for named paramaters feature... could we back to previous implementation and we can use func_num_args() instead of count($args) to detect number of args.

[deminy]

@yespire @twose I'd suggest rewrite it like following:

diff --git a/src/core/Database/PDOStatementProxy.php b/src/core/Database/PDOStatementProxy.php
index 0b2972b..1ec475b 100644
--- a/src/core/Database/PDOStatementProxy.php
+++ b/src/core/Database/PDOStatementProxy.php
@@ -123,13 +123,13 @@ class PDOStatementProxy extends ObjectProxy
         return $this->__object->setAttribute($attribute, $value);
     }
 
-    public function setFetchMode(int $mode, $classNameObject = null, array $ctorarfg = []): bool
+    /**
+     * @see https://php.net/pdostatement.setfetchmode
+     */
+    public function setFetchMode(int $mode): bool
     {
-        $this->setFetchModeContext = [$mode, $classNameObject, $ctorarfg];
-        if (!isset($classNameObject)) {
-            return $this->__object->setFetchMode($mode);
-        }
-        return $this->__object->setFetchMode($mode, $classNameObject, $ctorarfg);
+        $this->setFetchModeContext = func_get_args();
+        return $this->__object->setFetchMode(...$this->setFetchModeContext);
     }
 
     public function bindParam($parameter, &$variable, $data_type = PDO::PARAM_STR, $length = null, $driver_options = null): bool

Feel free to create a new PR instead if it looks good. Thanks

[yespire]

1. Add ignore files to your own user ignore list is better, or do this in another PR.

I will move the ignore change to a separate PR

2. I am so sorry that I recongnized that my idea is bad for PHP-8.x, it will break support for named paramaters feature... could we back to previous implementation and we can use func_num_args() instead of count($args) to detect number of args.

sure

[yespire]

@deminy Nice!

[twose]

The function signature in PHP8 is

/** @return bool */
public function setFetchMode(int $mode, mixed ...$args) {}

See: https://github.com/php/php-src/blob/master/ext/pdo/pdo_stmt.stub.php#L64

[yespire]

The function signature in PHP8 is

/** @return bool */
public function setFetchMode(int $mode, mixed ...$args) {}

See: https://github.com/php/php-src/blob/master/ext/pdo/pdo_stmt.stub.php#L64

Good catch
however if we add mixed type hinting, it will break in 7.X, mixed is added in php8 - https://wiki.php.net/rfc/mixed_type_v2

is "mixed" typing hinting a hard requirement ?

public function setFetchMode(int $mode, ...$args) : bool {
    $this->setFetchModeContext = [$mode, ...$args];
    return $this->__object->setFetchMode(...$this->setFetchModeContext);
}

[twose]

public function setFetchMode(int $mode, ...$args) : bool {
$this->setFetchModeContext = [$mode, ...$args];
return $this->__object->setFetchMode(...$this->setFetchModeContext);
}

It's ok to drop mixed. In my understanding, mixed means we did not forget to declare type of param, the param accept all kinds of values indeed.

[deminy]

I agree with @twose . This is better than what I suggested:

public function setFetchMode(int $mode, ...$args) : bool;

[deminy]

The PR has been merged. @yespire thanks for your contribution!