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

Bug: TypeError in pg_ping() #9043

Open
warcooft opened this issue Jul 10, 2024 · 7 comments · May be fixed by #9279
Open

Bug: TypeError in pg_ping() #9043

warcooft opened this issue Jul 10, 2024 · 7 comments · May be fixed by #9279
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@warcooft
Copy link
Contributor

warcooft commented Jul 10, 2024

PHP Version

8.3

CodeIgniter4 Version

4.5.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

apache

Database

PostgreSQL 16.3

What happened?

pg_ping() expects the first parameter to be a PostgreSQL connection (or null), but receives the boolean value false.

See also codeigniter4/queue#45 (comment)

This is a blocker for other packages.

Screenshot 2024-07-11 at 05 47 49

Steps to Reproduce

$db = db_connect();
d($db->connect());
d($db->close());
d($db->reconnect());
d($db->getDatabase());
d($db->getVersion());
dd();

Expected Output

The output should show the db name and db driver version.

Anything else?

No response

@warcooft warcooft added the bug Verified issues on the current code behavior or pull requests that will fix them label Jul 10, 2024
@warcooft
Copy link
Contributor Author

it looks like the close() function also doesn't work when using the PostgresSQL driver:

Screenshot 2024-07-11 at 07 24 15

 
this when using MySQL:

Screenshot 2024-07-11 at 07 25 22

Steps to Reproduce

$db = db_connect();
d($db);
// d($db->connect());
d($db->close());
// d($db->reconnect());
d($db->getPlatform());
d($db->getVersion());
dd();

@warcooft
Copy link
Contributor Author

and both connections cannot be closed if before closing we run connect() as follows:

$db = db_connect();
d($db);
d($db->connect());
d($db->close());
// d($db->reconnect());
d($db->getPlatform());
d($db->getVersion());
dd();

@warcooft warcooft changed the title Bug: TypeError: pg_ping(): Argument #1 ($connection) must be of type ?PgSql\Connection, false given Bug: TypeError in pg_ping() Jul 11, 2024
@paulbalandan
Copy link
Member

I can reproduce the TypeError in the original issue.

@paulbalandan
Copy link
Member

Dumb question: if you call reconnect() on a closed DB connection, should you initialize again? In the Postgres driver, you only call pg_ping() which just pings the connection if still active and tries to reconnect if broken. For MySQL driver, it explicitly closes then reinitializes the connection. Clearly a difference in behaviors, I suppose.

@warcooft
Copy link
Contributor Author

For MySQL driver, it explicitly closes then reinitializes the connection. Clearly a difference in behaviors.

I'm not sure if the way used to produce the error above is correct. I think both have the same behavior. thankyou for enlighten me.

@kenjis
Copy link
Member

kenjis commented Jul 25, 2024

The behavior of reconnect() is to keep the connection alive or re-establish it.

If the database server’s idle timeout is exceeded while you’re doing some heavy PHP lifting (processing an image, for instance), you should consider pinging the server by using the reconnect() method before sending further queries, which can gracefully keep the connection alive or re-establish it.
https://codeigniter4.github.io/CodeIgniter4/database/connecting.html#reconnecting-keeping-the-connection-alive

So if there is a ping method in the DB module, we should use it.

But in MySQL, ping() is no longer working. See #462

@warcooft
Copy link
Contributor Author

That helps! thanks for explaining.

@ping-yee ping-yee linked a pull request Nov 14, 2024 that will close this issue
5 tasks
@michalsn michalsn linked a pull request Nov 14, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants