Remove timeout translation from NpgsqlReadBuffer - #6126
#6126Remove timeout translation from NpgsqlReadBuffer#6126vonzshik merged 1 commit intomainnpgsql/npgsql:mainfrom 6122-read-buffer-timeout-translation-fixnpgsql/npgsql:6122-read-buffer-timeout-translation-fixCopy head branch name to clipboard
Conversation
180f367 to
50939cc
Compare
| ReadBuffer.Timeout = TimeSpan.FromSeconds(command?.CommandTimeout ?? Settings.CommandTimeout); | ||
| // But the next time, we call the Prepare, which doesn't set its own timeout | ||
| var timeoutSeconds = command?.CommandTimeout ?? Settings.CommandTimeout; | ||
| ReadBuffer.Timeout = timeoutSeconds > 0 ? TimeSpan.FromSeconds(timeoutSeconds) : Timeout.InfiniteTimeSpan; |
There was a problem hiding this comment.
Just confirming: from the user's perspective, we're retaining the meaning of CommandTimeout=0 to mean infinite/no timeout, right? (both in the connection string and in NpgsqlCommand.CommandTimeout). In those contexts, and "immediate timeout" makes no sense in any case.
(It's just slightly confusing that we have two contexts/meanings: one user-facing one where 0 means infinite and another internal one where it means immediate. I get why it's like this and it makes sense - maybe good to make it slightly clearer in comments here and there)
There was a problem hiding this comment.
Just confirming: from the user's perspective, we're retaining the meaning of CommandTimeout=0 to mean infinite/no timeout, right? (both in the connection string and in NpgsqlCommand.CommandTimeout). In those contexts, and "immediate timeout" makes no sense in any case.
Correct, that's why we pass Timeout.InfiniteTimeSpan if command's timeout is 0, since now instead of NpgsqlReadBuffer doing that for us, we have to do that ourselves at each call. Not ideal but then our COPY operations allow an essentially direct access to read buffer's timeout, and it's only two places, so...
Fixes #6122