php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #81464 Regression: pgsql resource types
Submitted: 2021-09-21 16:18 UTC Modified: 2021-09-23 12:40 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: weierophinney@php.net Assigned:
Status: Wont fix Package: PostgreSQL related
PHP Version: 8.1.0RC2 OS: Ubuntu 20.04
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: weierophinney@php.net
New email:
PHP Version: OS:

 

 [2021-09-21 16:18 UTC] weierophinney@php.net
Description:
------------
In all prior releases of PHP, the Postgres resources were as stated on https://www.php.net/manual/en/resource.php

With the merging of https://github.com/php/php-src/pull/6791, PHP 8.1.0 is changing pgsql resources into resource objects. This breaks existing code that is checking for the old resource types as currently documented. 

As such, this is a BC BREAK in a minor version.

Test script:
---------------
$pgsql = pg_connect($validConnectionString);

echo get_resource_type($pgsql), "\n";

$pgResult = pg_query($pgsql, $someValidSql);

echo get_resource_type($pgResult), "\n";

Expected result:
----------------
pgsql link
pgsql result

Actual result:
--------------
PgSql\Connection
PgSql\Result

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-09-21 16:21 UTC] girgias@php.net
-Status: Open +Status: Not a bug
 [2021-09-21 16:21 UTC] girgias@php.net
This is expected, is documented in UPGRADING, and will be documented in the migration guide.
 [2021-09-21 17:18 UTC] weierophinney@php.net
-Status: Not a bug +Status: Re-Opened
 [2021-09-21 17:18 UTC] weierophinney@php.net
It may be in UPGRADING, but that doesn't change the fact that a BC break is being introduced in a MINOR version. The API should be stable for the entirety of a major release, and these sorts of things should be introduced with 9.0, not 8.1.

I ask the core team to please reconsider these, as well as other related resource -> resource object changes made in the 8.1 release (finfo, ldap, and pspell, among others). These changes are (a) difficult to trace, (b) need to be called out better, and (c) should be limited to new major versions.

As an example, the laminas-db integration tests failed to discover the change... because they were skipping tests based on whether or not a "pgsql link" resource was present. The end result is that we thought we'd tested our postgres functionality against 8.1, and were in fact skipping those same tests, due to the fact that a change in the API masked the issue. We'd not thought there were any such changes because:

- There was no RFC related to 8.1 that indicated these changes were happening.
- It was a new minor version, so we wouldn't expect an API change.

We've adapted our code to accommodate the change now, but we shouldn't have needed to until 9.0.
 [2021-09-21 21:14 UTC] cmb@php.net
Could you please raise that issue on the internals mailing list?
This bugtracker is a poor medium for discussion, and visibility of
tickets is pretty poor, either.
 [2021-09-23 12:40 UTC] nikic@php.net
-Status: Re-Opened +Status: Wont fix
 [2021-09-23 12:40 UTC] nikic@php.net
An internals thread has been started at https://externals.io/message/116127.

Independently of the general policy discussion, I'm closing this particular issue as Won't Fix, because it's not possible to revert such changes this late in the release cycle. Even if we ignored release stability concerns, this is an ABI breaking change, and ABI is frozen since RC1.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 10:01:28 2024 UTC