Add a connection-independent pgoutput to ChangeEvent decoder#102
Add a connection-independent pgoutput to ChangeEvent decoder#102LucaCappelletti94 wants to merge 1 commit into
ChangeEvent decoder#102Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the logical replication message decoding logic by moving the core translation function message_to_change_event and its helpers from src/stream.rs to src/protocol.rs. It also introduces PgOutputDecoder to enable connection-independent decoding of pgoutput messages. The review feedback points out several opportunities to optimize performance and avoid unnecessary string allocations by passing owned String variables (prefix and gid) directly into functions that accept Into<Arc<str>>.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ChangeEvent::message( | ||
| flags, | ||
| Lsn::new(msg_lsn), | ||
| Arc::<str>::from(prefix.as_str()), |
There was a problem hiding this comment.
Since prefix is an owned String and ChangeEvent::message accepts any type implementing Into<Arc<str>>, you can pass prefix directly instead of creating a new Arc from a borrowed string slice. This avoids an unnecessary reallocation and string copy on the message decoding path.
| Arc::<str>::from(prefix.as_str()), | |
| prefix, |
There was a problem hiding this comment.
Applied the proposed cleanup
| Lsn::new(prepare_lsn), | ||
| Lsn::new(end), | ||
| postgres_timestamp_to_chrono(timestamp), | ||
| Arc::<str>::from(gid.as_str()), |
There was a problem hiding this comment.
Since gid is an owned String and ChangeEvent::begin_prepare accepts impl Into<Arc<str>>, you can pass gid directly to avoid an unnecessary reallocation and string copy.
| Arc::<str>::from(gid.as_str()), | |
| gid, |
There was a problem hiding this comment.
Applied the proposed cleanup
| Lsn::new(prepare_lsn), | ||
| Lsn::new(end), | ||
| postgres_timestamp_to_chrono(timestamp), | ||
| Arc::<str>::from(gid.as_str()), |
There was a problem hiding this comment.
Since gid is an owned String and ChangeEvent::prepare accepts impl Into<Arc<str>>, you can pass gid directly to avoid an unnecessary reallocation and string copy.
| Arc::<str>::from(gid.as_str()), | |
| gid, |
There was a problem hiding this comment.
Applied the proposed cleanup
| Lsn::new(commit_lsn), | ||
| Lsn::new(end), | ||
| postgres_timestamp_to_chrono(timestamp), | ||
| Arc::<str>::from(gid.as_str()), |
There was a problem hiding this comment.
Since gid is an owned String and ChangeEvent::commit_prepared accepts impl Into<Arc<str>>, you can pass gid directly to avoid an unnecessary reallocation and string copy.
| Arc::<str>::from(gid.as_str()), | |
| gid, |
There was a problem hiding this comment.
Applied the proposed cleanup
| Lsn::new(rollback_end_lsn), | ||
| postgres_timestamp_to_chrono(prepare_timestamp), | ||
| postgres_timestamp_to_chrono(rollback_timestamp), | ||
| Arc::<str>::from(gid.as_str()), |
There was a problem hiding this comment.
Since gid is an owned String and ChangeEvent::rollback_prepared accepts impl Into<Arc<str>>, you can pass gid directly to avoid an unnecessary reallocation and string copy.
| Arc::<str>::from(gid.as_str()), | |
| gid, |
There was a problem hiding this comment.
Applied the proposed cleanup
| Lsn::new(prepare_lsn), | ||
| Lsn::new(end), | ||
| postgres_timestamp_to_chrono(timestamp), | ||
| Arc::<str>::from(gid.as_str()), |
There was a problem hiding this comment.
Since gid is an owned String and ChangeEvent::stream_prepare accepts impl Into<Arc<str>>, you can pass gid directly to avoid an unnecessary reallocation and string copy.
| Arc::<str>::from(gid.as_str()), | |
| gid, |
There was a problem hiding this comment.
Applied the proposed cleanup
Merging this PR will improve performance by 5.51%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | copy_from_slice[64] |
1.7 µs | 1.6 µs | +5.51% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing LucaCappelletti94:feat/pgoutput-decoder (4ebcb96) with main (784e644)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
+ Coverage 94.53% 94.55% +0.01%
==========================================
Files 27 27
Lines 20179 20278 +99
==========================================
+ Hits 19077 19174 +97
- Misses 1102 1104 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
40703ce to
4ebcb96
Compare
1 participant
The step that turns a parsed pgoutput message into a
ChangeEventlived as a private method on the network-onlyLogicalReplicationStream, inside a module gated behind a network backend. The only way to obtain aChangeEventwas to open a live replication connection and drive the high-levelEventStream. That is a poor fit for consumers that hold no connection, such as a tests or a fuzz harness. This change lifts the assembly into the unconditionalprotocolmodule so those consumers can decode with only bytes in hand.This is a pure refactor with no decode behavior change. The large code changes are just shifting the necessary method between two modules.