-
Notifications
You must be signed in to change notification settings - Fork 104
[Electric]: Use numbers for txid #245
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
Conversation
🦋 Changeset detectedLatest commit: 75a86ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@tanstack/db-example-react-todo
commit: |
Size Change: 0 B Total Size: 31.4 kB ℹ️ View Unchanged
|
Size Change: 0 B Total Size: 1.05 kB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Note how much easier this would have been if we had used a type alias type TxId = number
. Perhaps this is a good time to introduce it?
} | ||
|
||
return String(txid) | ||
return parseInt(txid, 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps good to check that the parsed int is not NaN
? Just to safeguard against that even though it doesn't look very plausible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah not sure pg could return an unparsable number? Though I did some more research and txid_current()
is actually deprecated. But with the new function, we can cast it to xid then to text and then parse it in js to get the 32bit internal xid that's also on the logical replication:
async function generateTxId(tx: any): Promise<Txid> {
// The ::xid cast strips off the epoch, giving you the raw 32-bit value
// that matches what PostgreSQL sends in logical replication streams
// (and then exposed through Electric which we'll match against
// in the client).
const result = await tx`SELECT pg_current_xact_id()::xid::text as txid`
const txid = result[0]?.txid
if (txid === undefined) {
throw new Error(`Failed to get transaction ID`)
}
return parseInt(txid, 10)
}
Fix #246
After some more research, this is the best way to get the 32bit internal xid (tx id) that is also sent out over logical replication and then through Electric i.e. what we'll match against inside Electric collections.