-
-
Notifications
You must be signed in to change notification settings - Fork 21
Need fix after amphp/sql update #17
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
Conversation
src/PgSqlResultSet.php
Outdated
| * @return int Number of fields in each row. | ||
| */ | ||
| public function numFields(): int | ||
| public function getNumFields(): int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be getFieldCount()?
src/PgSqlResultSet.php
Outdated
| * | ||
| * @throws \Error If the field number does not exist in the result. | ||
| */ | ||
| public function fieldName(int $fieldNum): string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to getFieldName()?
src/ResultSet.php
Outdated
| * @return int | ||
| */ | ||
| public function numFields(): int; | ||
| public function getNumFields(): int; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be added to Amp\Sql\ResultSet or does MySQL not provide this information upfront?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amp\Mysql has this: public function getFields(): Promise
But it's written with Amp\Socket, so we can probably refactor it.
resolves #15