Skip to content
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

'persistent' connection parameter neglected in postgre driver #349

Closed
groupnet opened this issue Jan 24, 2020 · 7 comments
Closed

'persistent' connection parameter neglected in postgre driver #349

groupnet opened this issue Jan 24, 2020 · 7 comments

Comments

@groupnet
Copy link
Contributor

Version: 4.x

Bug Description

The config parameter 'persistent' has no effect on how the Postgres connection is made.

Possible Solution

#348

@groupnet groupnet changed the title 'persistent' connection neglected in postgre driver 'persistent' connection parameter neglected in postgre driver Jan 24, 2020
@groupnet
Copy link
Contributor Author

One more comment: as the default behavior of pg_connect(...) is to establish a persistent connection, it might be a consideration to make postgre connections via dibi persistent per default too. In other words, only if the 'persistent' connection parameter is explicitly set to false, the connection is non-persistent (connection type PGSQL_CONNECT_FORCE_NEW is set).

If you prefer this breaking change to the bug fix above, I'd create a suitable pull request.

@dg
Copy link
Owner

dg commented Jan 24, 2020

closed by #348

@dg dg closed this as completed Jan 24, 2020
@dg
Copy link
Owner

dg commented Jan 24, 2020

I am confused about pg_connect vs pg_pconnect vs PGSQL_CONNECT_FORCE_NEW. It seems like something different https://github.com/php/php-src/blob/5d6564101192f8560d175cdec96c319d3c878b82/ext/pgsql/pgsql.c#L1358-L1442

@groupnet
Copy link
Contributor Author

Sorry, I've linked the wrong doc (I mentioned 'pg_connect' but linked docs of 'pg_pconnect').

In any case, I would stick to 'pg_connect(...)', because the latest comments about 'pg_pconnect(...)' at php.net mention that it is broken:

You should not use pg_pconnect - it's broken. It will work but it doesn't really pool, and it's behaviour is unpredictable. ...
(more details: https://www.php.net/manual/en/function.pg-pconnect.php#87550)

Anyway, what I wanted to point out in my original post above was pg_connect(...) seems to find and grab an existing connection anyway, but only if PGSQL_CONNECT_FORCE_NEW is not explicitly set:
https://github.com/php/php-src/blob/5d6564101192f8560d175cdec96c319d3c878b82/ext/pgsql/pgsql.c#L1442-L1456

So I was wondering if it wouldn't make more sense, to call pg_connect(...) without PGSQL_CONNECT_FORCE_NEW per default, like this:
groupnet@1e717ba

In a quick'n'dirty test this seems to speedup the re-connection significantly.

@dg
Copy link
Owner

dg commented Jan 24, 2020

I don't know, I've never used PostgreSQL. But I would not like to make a BC break in the patch version, so rather in 4.2.

@groupnet
Copy link
Contributor Author

I wouldn't either. ;)
I'll prepare a pull request (rebased to the current master) - feel free to merge it into 4.2 (or not).

Thanks for your good work!

@dg
Copy link
Owner

dg commented Jan 24, 2020

Ok, thanks.

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

No branches or pull requests

2 participants