Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes column rereading by tightening/streamlining PgReader seek/restart mechanics and adjusting GetBytes to avoid extra work when repositioning within a field, improving reread performance (notably alongside #6490’s restart behavior changes).
Changes:
- Update
NpgsqlDataReader.GetBytesto reuse a cachedPgReaderlocal, use the newIsFieldPastOffsetcheck, and clamp reads viaCurrentRemainingafter seeking. - Refactor
PgReaderseeking/restarting: rename offset-check helper, changeSeektovoid, and speed up restart rewinds via a lighter-weight rewind path. - Introduce a named sentinel constant for DB null field sizes and reuse cached
FieldSizeduring restart.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Npgsql/NpgsqlDataReader.cs | Adjusts GetBytes seeking/clamping to use cached reader state and updated PgReader APIs. |
| src/Npgsql/Internal/PgReader.cs | Refactors seek/restart internals for faster restarts and clearer sentinel usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR refines PgReader seeking/restarting behavior to reduce overhead in column rereads, complementing the restart behavior changes from #6490 to improve reread performance.
Changes:
- Adjust
NpgsqlDataReader.GetBytesto seek without relying on a return value and cap reads usingPgReader.CurrentRemaining. - Update
PgReader.Seekto returnvoidand streamline offset handling; add a low-levelRewindCorefor restart fast-path. - Minor internal cleanup: introduce
DbNullSentinelconstant and renameIsFieldConsumed→IsFieldPastOffset.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Npgsql/NpgsqlDataReader.cs | Updates GetBytes to use the new PgReader.Seek shape and compute remaining bytes via CurrentRemaining. |
| src/Npgsql/Internal/PgReader.cs | Makes Seek void, optimizes Restart rewind path, and clarifies internal sentinel/naming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| void RewindCore(int count) | ||
| { |
There was a problem hiding this comment.
Maybe we should add Debug.Assert here, just like Rewind has for CurrentOffset < count, _buffer.ReadPosition < count and StreamActive?
Together with the change to restart behavior in #6490 significantly improves column rereading perf (~10%).