Skip to content

pgsql ext: pg_query_params/pg_send_query_params false constant handli…#7651

Open
devnexen wants to merge 2 commits intophp:masterfrom
devnexen:postgres_query_params_bool_handling_fix
Open

pgsql ext: pg_query_params/pg_send_query_params false constant handli…#7651
devnexen wants to merge 2 commits intophp:masterfrom
devnexen:postgres_query_params_bool_handling_fix

Conversation

@devnexen
Copy link
Copy Markdown
Member

…ng proposal.

making a new subset of zend_get_string calls and concealed for the pgsql module for now.

@devnexen devnexen force-pushed the postgres_query_params_bool_handling_fix branch from 84a1c0e to 1f4239d Compare November 13, 2021 17:53
@Girgias
Copy link
Copy Markdown
Member

Girgias commented Nov 14, 2021

I really don't think adding a Zend API for this case is the way to go, when you can just to a check against zend_empty_string in PGSQL to branch of.

@devnexen
Copy link
Copy Markdown
Member Author

I thought of something like this was more in case later it can be used beyond pgsql ext.

@devnexen devnexen force-pushed the postgres_query_params_bool_handling_fix branch from 1f4239d to ebfaeea Compare November 14, 2021 15:02
@devnexen
Copy link
Copy Markdown
Member Author

ping :)

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Mar 30, 2022

Thanks for the PR! I'm not quite sure what to do here; the issue had been reported as https://bugs.php.net/68156, and re-classified as feature request back then. And at least the fix in this PR appears to introduce a BC break; consider using str instead of num in the when-clause; passing false would no longer match an empty string. I think we cannot change that behavior for any stable release; it might be okay to change it for master, but even that might be debatable.

Also, someone mentioned in that bug report that PDO_PGSQL would behave the same (I haven't checked whether this is still the case), so it might be reasonable to change its behavior as well.

BTW, I just noticed that the pgsql test suite is pretty volatile regarding the setup of the test tables; unless 01createdb.phpt is run, most other tests would fail since there is no explicit table creation.

@devnexen devnexen changed the base branch from PHP-8.0 to master April 1, 2022 17:51
@devnexen devnexen force-pushed the postgres_query_params_bool_handling_fix branch 3 times, most recently from 858b38e to d17bd4a Compare April 1, 2022 19:44
@devnexen devnexen requested a review from SakiTakamachi as a code owner April 15, 2024 15:43
@Girgias
Copy link
Copy Markdown
Member

Girgias commented Apr 6, 2026

I think with our new policy we can ValueError on invalid types.

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.

3 participants