Skip to content

Fetching rows as native arrays for better static analysis - fetchAllAssoc - #337

Open
wonka007 wants to merge 2 commits into
nette:masterfrom
wonka007:fetch-all-assoc
Open

wonka007 wants to merge 2 commits into
nette:masterfrom
wonka007:fetch-all-assoc

Conversation

@wonka007

Copy link
Copy Markdown
  • bug fix / new feature? Allows rows to be fetched directly as arrays as fetchAssoc does.
  • BC break? NO
  • doc PR: nette/docs#??? (Will provide after I know that there is interest for this feature.)

Apply the possibility to use fetchAssoc to get native array type of next row to fetchAll without the need to convert from \Nette\Database\Row back to array.

\Nette\Database\Row cannot be typed properly (at least I wasn't able to use something like

/** @var \Nette\Database\Row{id: int} */

But this syntax is possible to be used with fetchAssoc because that returns array:

/** @var array{id: int, name: string} */
$this->db->fetchAssoc("SELECT id, name FROM user WHERE id = ?", $user_id);

This new proposed function fetchAllAssoc copies what fetchAll does but without the need to convert array that are retrieved from the driver into \Nette\Database\Row (Arrays::toObject($data, new Row)) while not breaking the iterator over rows with original type.

The result is list which can be typed similarly as example above.

/** @var list<array{id: int, name: string}> */
$this->db->fetchAllAssoc("SELECT id, name FROM user");

This appoach unclocks new possibilities for static analysis. We have to keep in mind that these hints rely on developer knowledge of the query string used and will likely not be useful for higly dynamic queries where unions would have to be used and checked.

It also saves some performance because convertion to \Nette\Database\Row is not useful most of the time.


Duplicate PR because the old was was done for older version (3.3) rather then master and naming was not consistent.
Not pulling to 3.3 because future version looks like it will contain big changes (combining Connection and Explorer) so it is more appropriate to apply this change to future version only.
This PR also fix small coding style fixes. Big finds were ignored for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant