Skip to content

oracledb_cdc: ensure LogMiner nullable numbers are consistent with snapshotting#4354

Merged
josephwoodward merged 4 commits intomainfrom
jw/oracledbnullstr
Apr 28, 2026
Merged

oracledb_cdc: ensure LogMiner nullable numbers are consistent with snapshotting#4354
josephwoodward merged 4 commits intomainfrom
jw/oracledbnullstr

Conversation

@josephwoodward
Copy link
Copy Markdown
Contributor

@josephwoodward josephwoodward commented Apr 28, 2026

This change ensures nullable numbers are treated the same as with snapshotting and represented as null.

Change adds a unit test and also an integration test to verify handling is consistent with snapshotting.

This change ensures nullable numbers are treated the same as with
snapshotting and represented as null.
Comment thread internal/impl/oracledb/logminer/sqlredo/parser.go Outdated
@claude
Copy link
Copy Markdown

claude Bot commented Apr 28, 2026

Commits
LGTM

Review
Single-commit PR fixing nullable number handling in oracledb_cdc so streamed events match snapshot output (NULLs surface as JSON null rather than being dropped from the map). Test coverage is added at the parser level and exercised end-to-end in the integration test.

  1. Stale comments and a commented-out line of source remain at parser.go#L195-L204 that describe the old "exclude from map" behavior. See inline comment.

@josephwoodward josephwoodward marked this pull request as ready for review April 28, 2026 12:01
@josephwoodward josephwoodward changed the title oracledb_cdc: ensure nullable numbers consistent with snapshotting oracledb_cdc: ensure nullable numbers are consistent with snapshotting Apr 28, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 28, 2026

Commits
LGTM

Review
Small, focused fix to ensure NULL nullable numbers flow through as JSON null rather than being dropped from the message map during streaming. Changes are well-tested via both the unit tests in parser_test.go and the expanded integration test asserting NULLABLE_NUM/NONNULLABLE_NUM behavior across snapshot and streaming.

LGTM

@claude
Copy link
Copy Markdown

claude Bot commented Apr 28, 2026

Commits

  1. Several commits (oracledb_cdc: remove redundant comment, oracledb_cdc: add test case for non nullable numbers, oracledb_cdc: address linting issue) appear to be incremental fixups of the main change in oracledb_cdc: ensure nullable numbers consistent with snapshotting and have empty bodies. The test case for non-nullable numbers is logically part of the fix it tests, and the linting fix addresses code introduced in the same series — these would be cleaner squashed into the main commit. See commit policy: each commit should be one small, self-contained, logical change.

Review
The fix correctly addresses the inconsistency where NULL columns from streaming were silently omitted from the event map while snapshot output included them as JSON nulls. The lowercase "null" check at parser.go#L201-L204 addresses a real vitess-sqlparser serialization quirk, and unit + integration tests cover both nullable and non-nullable cases.

LGTM

@josephwoodward josephwoodward changed the title oracledb_cdc: ensure nullable numbers are consistent with snapshotting oracledb_cdc: ensure LogMiner nullable numbers are consistent with snapshotting Apr 28, 2026
@josephwoodward josephwoodward merged commit 3bbbbea into main Apr 28, 2026
7 checks passed
@josephwoodward josephwoodward deleted the jw/oracledbnullstr branch April 28, 2026 12:36
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.

2 participants