Skip to content

mysql_cdc: ensure snapshot primary key order is consistent for composite primary keys#4262

Merged
josephwoodward merged 1 commit intomainfrom
jw/snapshotmaporder
Apr 15, 2026
Merged

mysql_cdc: ensure snapshot primary key order is consistent for composite primary keys#4262
josephwoodward merged 1 commit intomainfrom
jw/snapshotmaporder

Conversation

@josephwoodward
Copy link
Copy Markdown
Contributor

@josephwoodward josephwoodward commented Apr 15, 2026

Go's map do not guarantee ordering (the Go team even intentionally build randomisation into it to dissuade people from depending on its order), so switch to iterating ordered pk slice instead.

image

Closes: #4256

@josephwoodward josephwoodward changed the title mysql_cdc: ensure snapshot column order is consistent for composite primary keys mysql_cdc: ensure snapshot primary key order is consistent for composite primary keys Apr 15, 2026
@claude
Copy link
Copy Markdown

claude bot commented Apr 15, 2026

Commits
LGTM

Review
Clean bug fix: querySnapshotTable was iterating over a map[string]any (*lastSeenPkVal) directly, which yields non-deterministic key order in Go. For composite primary keys, this caused the values passed to the WHERE (col1, col2) > (?, ?) clause to potentially be in the wrong order relative to the column names from pk, resulting in incorrect snapshot pagination. The fix iterates over the ordered pk slice and looks up each value by name, ensuring consistent column-value alignment. Covered by existing integration test TestIntegrationMySQLCDCWithCompositePrimaryKeys.

LGTM

Comment thread internal/impl/mysql/snapshot.go Outdated
@claude
Copy link
Copy Markdown

claude bot commented Apr 15, 2026

Commits
LGTM

Review
Single-file bug fix in internal/impl/mysql/snapshot.go that switches snapshot pagination from iterating over a map (non-deterministic order in Go) to iterating over the pk slice, ensuring primary key column values are always in the correct order when constructing the paginated query. The missing-key error path is handled properly.

LGTM

Copy link
Copy Markdown
Contributor

@mmatczuk mmatczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@josephwoodward josephwoodward merged commit 224032c into main Apr 15, 2026
7 checks passed
@josephwoodward josephwoodward deleted the jw/snapshotmaporder branch April 15, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mysql_cdc Composite PK cursor wrong order (snapshot.go:197)

3 participants