Skip to content
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

add object shape for mysqli_result::fetch_fields #3005

Merged

Conversation

schlndh
Copy link
Contributor

@schlndh schlndh commented Apr 6, 2024

See https://www.php.net/manual/en/mysqli-result.fetch-field.php and https://www.php.net/manual/en/mysqli-result.fetch-fields.php (it's incomplete, but it does mention the change from max_length: int to max_length: 0 in PHP 8.1).

Here is an example field I got from PHP 8.3 via mysqli_result::fetch_fields:

  class stdClass#3665 (13) {
    public $name =>
    string(4) "name"
    public $orgname =>
    string(4) "name"
    public $table =>
    string(13) "analyser_test"
    public $orgtable =>
    string(13) "analyser_test"
    public $def =>
    string(0) ""
    public $db =>
    string(14) "mariastan_test"
    public $catalog =>
    string(3) "def"
    public $max_length =>
    int(0)
    public $length =>
    int(1020)
    public $charsetnr =>
    int(45)
    public $flags =>
    int(0)
    public $type =>
    int(253)
    public $decimals =>
    int(0)
  }

Here is the php-src code that seems to be responsible for populating the stdClass (at least as far as I can tell): https://github.com/php/php-src/blob/cf313321c2e13319d479e0dd4f49094cc72cf652/ext/mysqli/mysqli_api.c#L650

@staabm
Copy link
Contributor

staabm commented Apr 6, 2024

You might get better types with https://github.com/staabm/phpstan-dba in case you wanne give it a try

Regarding this PR. We need to make sure this api returns this array shapes across mysql and mariadb versions.

@schlndh
Copy link
Contributor Author

schlndh commented Apr 6, 2024

@staabm Thanks for the tip. I am aware of your extension, and I have my own: https://github.com/schlndh/maria-stan . Extensions which understand the query can of course provide a better return type. But it's still worth it to have a reasonable return type directly in phpstan.

As for mysql vs mariadb: The example I have given is with MariaDB 10.11, but based on the php-src link that I provided in the PR description I believe the type to be accurate both for mariadb and mysql (as well as either of those with libmysqlclient instead of mysqlnd).

I have not tried it myself across a range of DBs, so it's possible that I missed something in php-src. But IMO it's not worth the effort to try and verify it experimentally, given that I'm reasonably confident in the type being correct. The worst case is that someone reports a bug and the type will be amended accordingly. 🤷‍♂️

@@ -21,7 +21,12 @@
*/
return [
'new' => [

'mysqli_fetch_field' => ['(stdClass&object{name: string, orgname: string, table: string, orgtable: string, def: string, db: string, catalog: "def", max_length: 0, length: int, charsetnr: string, flags: int, type: int, decimals: int})|false', 'result'=>'mysqli_result'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's different about these on 8.1+?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max_length is always zero starting from PHP 8.1, as documented in https://www.php.net/manual/en/mysqli-result.fetch-fields.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the commit which changes it to be always 0: php/php-src@890e4ca

'mysqli_result::fetch_field_direct' => ['object|false', 'fieldnr'=>'int'],
'mysqli_result::fetch_fields' => ['array'],
'mysqli_result::fetch_field' => ['(stdClass&object{name: string, orgname: string, table: string, orgtable: string, def: string, db: string, catalog: "def", max_length: int, length: int, charsetnr: string, flags: int, type: int, decimals: int})|false'],
'mysqli_result::fetch_field_direct' => ['(stdClass&object{name: string, orgname: string, table: string, orgtable: string, def: string, db: string, catalog: "def", max_length: int, length: int, charsetnr: string, flags: int, type: int, decimals: int})|false', 'fieldnr'=>'int'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either the PHP docs are wrong, or this PR is wrong. Looking at this https://www.php.net/manual/en/mysqli-result.fetch-field-direct.php it's not exactly 1:1. For example db and catalog fields are missing in PHP docs. Can you please verify the return types are the same, and send a PR to PHP docs? Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell based on the php_add_field_properties function linked in the PR description and by trying the 3 functions/methods myself: the PR is right, the docs are wrong.

<?php

declare(strict_types=1);

$db = new mysqli(
	'127.0.0.1',
	'root',
	'root'
);
$result = $db->query('SELECT 1');
var_dump($result->fetch_fields());
var_dump($result->fetch_field());
var_dump($result->fetch_field_direct(0));

$result = $db->query('SELECT 1');
var_dump(mysqli_fetch_fields($result));
var_dump(mysqli_fetch_field($result));
var_dump(mysqli_fetch_field_direct($result, 0));

Gives the following result (PHP 8.3, MariaDB 10.11):

array(1) {
  [0]=>
  object(stdClass)#3 (13) {
    ["name"]=>
    string(1) "1"
    ["orgname"]=>
    string(0) ""
    ["table"]=>
    string(0) ""
    ["orgtable"]=>
    string(0) ""
    ["def"]=>
    string(0) ""
    ["db"]=>
    string(0) ""
    ["catalog"]=>
    string(3) "def"
    ["max_length"]=>
    int(0)
    ["length"]=>
    int(1)
    ["charsetnr"]=>
    int(63)
    ["flags"]=>
    int(32897)
    ["type"]=>
    int(3)
    ["decimals"]=>
    int(0)
  }
}
object(stdClass)#3 (13) {
  ["name"]=>
  string(1) "1"
  ["orgname"]=>
  string(0) ""
  ["table"]=>
  string(0) ""
  ["orgtable"]=>
  string(0) ""
  ["def"]=>
  string(0) ""
  ["db"]=>
  string(0) ""
  ["catalog"]=>
  string(3) "def"
  ["max_length"]=>
  int(0)
  ["length"]=>
  int(1)
  ["charsetnr"]=>
  int(63)
  ["flags"]=>
  int(32897)
  ["type"]=>
  int(3)
  ["decimals"]=>
  int(0)
}
object(stdClass)#3 (13) {
  ["name"]=>
  string(1) "1"
  ["orgname"]=>
  string(0) ""
  ["table"]=>
  string(0) ""
  ["orgtable"]=>
  string(0) ""
  ["def"]=>
  string(0) ""
  ["db"]=>
  string(0) ""
  ["catalog"]=>
  string(3) "def"
  ["max_length"]=>
  int(0)
  ["length"]=>
  int(1)
  ["charsetnr"]=>
  int(63)
  ["flags"]=>
  int(32897)
  ["type"]=>
  int(3)
  ["decimals"]=>
  int(0)
}
array(1) {
  [0]=>
  object(stdClass)#2 (13) {
    ["name"]=>
    string(1) "1"
    ["orgname"]=>
    string(0) ""
    ["table"]=>
    string(0) ""
    ["orgtable"]=>
    string(0) ""
    ["def"]=>
    string(0) ""
    ["db"]=>
    string(0) ""
    ["catalog"]=>
    string(3) "def"
    ["max_length"]=>
    int(0)
    ["length"]=>
    int(1)
    ["charsetnr"]=>
    int(63)
    ["flags"]=>
    int(32897)
    ["type"]=>
    int(3)
    ["decimals"]=>
    int(0)
  }
}
object(stdClass)#2 (13) {
  ["name"]=>
  string(1) "1"
  ["orgname"]=>
  string(0) ""
  ["table"]=>
  string(0) ""
  ["orgtable"]=>
  string(0) ""
  ["def"]=>
  string(0) ""
  ["db"]=>
  string(0) ""
  ["catalog"]=>
  string(3) "def"
  ["max_length"]=>
  int(0)
  ["length"]=>
  int(1)
  ["charsetnr"]=>
  int(63)
  ["flags"]=>
  int(32897)
  ["type"]=>
  int(3)
  ["decimals"]=>
  int(0)
}
object(stdClass)#2 (13) {
  ["name"]=>
  string(1) "1"
  ["orgname"]=>
  string(0) ""
  ["table"]=>
  string(0) ""
  ["orgtable"]=>
  string(0) ""
  ["def"]=>
  string(0) ""
  ["db"]=>
  string(0) ""
  ["catalog"]=>
  string(3) "def"
  ["max_length"]=>
  int(0)
  ["length"]=>
  int(1)
  ["charsetnr"]=>
  int(63)
  ["flags"]=>
  int(32897)
  ["type"]=>
  int(3)
  ["decimals"]=>
  int(0)
}

I will see about updating the PHP documentation, but it's not that simple because there are other inconsistencies and I don't have sufficient idea about the precise meaning of the fields.

@ondrejmirtes ondrejmirtes merged commit 28c5729 into phpstan:1.10.x Apr 7, 2024
435 of 444 checks passed
@ondrejmirtes
Copy link
Member

Thanks a lot!

@schlndh schlndh deleted the feature-mysqliResultFetchFields branch April 7, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants