Skip to content

Fix #78855: Native PHP types in database fetches #4939

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oleg-st
Copy link
Contributor

@oleg-st oleg-st commented Nov 22, 2019

Added PSQL_TYPED flag for pg_fetch_array and pg_fetch_all functions to return native typed arrays.
Converting to native type is almost the same as in pdo_pgsql.

@zbenc
Copy link

zbenc commented Nov 22, 2019

@oleg-st What about the Postgres types float/real and double precision? Those can be safely converted to PHP float as Postgres uses IEEE 754, just like PHP (of course numeric/decimal must remain string in PHP).

@lubosdz
Copy link

lubosdz commented Nov 22, 2019

Wish this could be done in mysqlnd. It would save lots of headaches, particularly since promoting features with strict typing.

@oleg-st
Copy link
Contributor Author

oleg-st commented Nov 22, 2019

@oleg-st What about the Postgres types float/real and double precision? Those can be safely converted to PHP float as Postgres uses IEEE 754, just like PHP (of course numeric/decimal must remain string in PHP).

Added floats.

@cmb69
Copy link
Member

cmb69 commented Nov 22, 2019

@oleg-st, since this is a new feature, and not a bug fix, it may only target PHP-7.4.

@lubosdz, feel free to provide a pull request. :)

@oleg-st
Copy link
Contributor Author

oleg-st commented Nov 22, 2019

@cmb69 PHP-7.4 is freezed for features, isn't it?

var_dump(pg_fetch_all($res, PGSQL_NUM|PGSQL_TYPED));

?>
--EXPECTF--
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be replaced by --EXPECT--

@cmb69
Copy link
Member

cmb69 commented Nov 22, 2019

@oleg-st, this feature might be regarded as small and self-contained addition, and as such might be permissible for PHP-7.4. If not, the PR should target master. Anyway, IMO this cannot go into PHP-7.2. which the PR currently targets.

@oleg-st
Copy link
Contributor Author

oleg-st commented Nov 23, 2019

@villfa Done
@cmb69 I can do it this PR or should create new one?

@cmb69
Copy link
Member

cmb69 commented Nov 23, 2019

@oleg-st, you can change the base-branch of this PR.

@oleg-st oleg-st changed the base branch from PHP-7.2 to PHP-7.4 November 23, 2019 08:02
codercms added a commit to codercms/ext-postgresql that referenced this pull request Apr 4, 2020
codercms added a commit to codercms/ext-postgresql that referenced this pull request Apr 4, 2020
According to this PR - php/php-src#4939
@oleg-st oleg-st changed the base branch from PHP-7.4 to master June 30, 2020 08:18
@oleg-st
Copy link
Contributor Author

oleg-st commented Jun 30, 2020

Changed base branch to master

@cmb69
Copy link
Member

cmb69 commented Dec 28, 2021

@oleg-st, you also need to rebase onto master, and force push; otherwise CI won't work as expected (especially, AppVeyor).

Regarding the actual feature, it might be a good idea to write to the internals mailing list for visibility.

@oleg-st
Copy link
Contributor Author

oleg-st commented Dec 28, 2021

@cmb69 Rebased

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