-
Notifications
You must be signed in to change notification settings - Fork 183
fix(postgres): support replication slot names containing numbers (#679) #741
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
base: master
Are you sure you want to change the base?
fix(postgres): support replication slot names containing numbers (#679) #741
Conversation
Fixes datazip-inc#679 Updated SQL queries to use parameterized queries ($1, $2) instead of string formatting with single quotes. This resolves the syntax error that occurred when replication slot names started with numbers (e.g., '123_olake_slot'). Changes: - Updated ReplicationSlotTempl to ReplicationSlotQuery with $1 parameter - Updated AdvanceLSNTemplate to AdvanceLSNQuery with $1, $2 parameters - Modified all usages to pass slot names as query parameters - Affects waljs.go, replicator.go, pgoutput.go, and cdc.go PostgreSQL replication slot names can contain underscores and numbers, and this fix ensures olake properly supports all valid slot names.
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.
Pull request overview
This pull request fixes a bug where PostgreSQL replication slot names starting with numbers (e.g., 123_olake_slot) caused syntax errors during CDC sync operations. The fix migrates from string formatting to parameterized queries for all replication slot-related SQL queries.
Key changes:
- Replaced
fmt.Sprintfstring formatting with parameterized queries ($1,$2) for all replication slot SQL operations - Renamed constants from
*Templateto*Queryto reflect the new parameterized approach - Updated all function calls to pass parameters directly to database methods instead of pre-formatting SQL strings
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pkg/waljs/replicator.go |
Updated ReplicationSlotQuery constant and getSlotPosition() and AdvanceLSN() functions to use parameterized queries |
pkg/waljs/waljs.go |
Updated AdvanceLSNQuery constant and wal2json replicator's StreamChanges() to use parameterized queries |
pkg/waljs/pgoutput.go |
Updated pgoutput replicator's StreamChanges() to use parameterized queries |
drivers/postgres/internal/cdc.go |
Updated validateReplicationSlot() to use parameterized queries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@hash-data please review and add appropriate label on behalf of swoc26 |
|
@SujalTripathi added thanks for pointing out |
|
Please review the PR @Akshay-datazip @hash-data |
|
@SujalTripathi Can you please upload a video of you showing your work by running a sync using OLake. You can record a video showing replication slot in Postgres containing numbers is used and the sync runs successfully. Also please rebase your PR from master to staging. After the video is uploaded and verified we will assign a reviewer as soon as we can. |
Description
Fixes issue where PostgreSQL replication slots with names starting with numbers (e.g.,
123_olake_slot) were causing syntax errors during CDC sync operations.Problem
The issue occurred because the code was using string formatting with single quotes (
'%s') for PostgreSQL identifiers. When a replication slot name starts with a number, PostgreSQL incorrectly interprets it as a string literal instead of an identifier, resulting in syntax errors [web:13][web:15].Solution
fmt.Sprintf) with parameterized queries ($1,$2) in SQL templatesReplicationSlotTemplto use$1parameter instead of'%s'AdvanceLSNTemplateto use$1and$2parametersThis approach provides multiple benefits:
Changes Made
pkg/waljs/replicator.go: Updated SQL query templates to use parameterized queriesdrivers/postgres/waljs/waljs.go: UpdatedGetReplicationSlot()andAdvanceLSN()to pass parameters correctlydrivers/postgres/pgoutput/pgoutput.go: Updated replication slot query calldrivers/postgres/cdc.go: Updated CDC validation to use parameterized queryTesting
go build ./...Related Issue
Closes #679
@hash-data