Skip to content

Use get_result instead of execute for pool connection ping#294

Merged
weiznich merged 1 commit into
diesel-rs:mainfrom
DhineshKrishnan1206:fix/pool-ping-read-response
Jun 19, 2026
Merged

Use get_result instead of execute for pool connection ping#294
weiznich merged 1 commit into
diesel-rs:mainfrom
DhineshKrishnan1206:fix/pool-ping-read-response

Conversation

@DhineshKrishnan1206

@DhineshKrishnan1206 DhineshKrishnan1206 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Changes the Verified recycling method's ping from .execute() to .get_result::<i32>() so the response row is actually read back from the server
  • This detects connections with corrupt internal read buffers that silently pass .execute() (which only reads the row count) but fail on real queries with "Unexpected end of row" / "buf doesn't have enough data" errors
  • Updates the corresponding trait bounds from ExecuteDsl to LoadQuery across all pool backends (bb8, deadpool, mobc) and the r2d2 connection wrapper

Fixes #139

Context

As discussed in #139, the current ping uses .execute("SELECT 1") which sends the query and reads the affected row count, but never reads actual row data. A connection whose read buffer contains leftover bytes from a previous partially-consumed response will pass this check, get returned to the pool, and then fail on the next real query.

Using .get_result::<i32>() forces the connection to deserialize a row, which will surface buffer corruption immediately during the health check rather than on a subsequent application query.

Test plan

  • cargo check --features "postgres bb8 deadpool mobc r2d2" — compiles cleanly
  • cargo check --features "mysql bb8 deadpool mobc" — compiles cleanly
  • cargo check --features "sqlite bb8 deadpool mobc r2d2" — compiles cleanly

@weiznich weiznich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks for submitting ❤️

This can be merged after a rebase on top of #293 as that one contains the fix for the failing CI job. I will take care of this later today.

The Verified recycling method now reads the result row from SELECT 1
via get_result::<i32>() instead of discarding it with execute(). This
detects connections with corrupt read buffers that would otherwise pass
the ping check but fail on subsequent queries.

Fixes diesel-rs#139
@weiznich
weiznich force-pushed the fix/pool-ping-read-response branch from 844ab4f to 66a6877 Compare June 19, 2026 06:58
@weiznich
weiznich merged commit 39987dc into diesel-rs:main Jun 19, 2026
25 checks passed
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.

The conneciton pool returns corrupt connections

2 participants