Skip to content

fix(connector): preserve SQL Server composite primary key order - #27160

Merged
zwang28 merged 1 commit into
mainfrom
check-sqlserver-cdc-issue
Sep 18, 2026
Merged

zwang28 merged 1 commit into
mainfrom
check-sqlserver-cdc-issue

Conversation

@zwang28

@zwang28 zwang28 commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • I have checked the Release Timeline and Currently Supported Versions to determine which release branches I need to cherry-pick this PR into.

Documentation

  • My PR needs documentation updates.
Release note

Summary by CodeRabbit

  • Bug Fixes

    • Improved SQL Server CDC handling for tables with composite primary keys.
    • Preserved the source table’s column and primary-key ordering during replication.
    • Improved consistency when mapping and querying replicated rows from composite-key tables.
  • Tests

    • Added coverage for composite primary-key tables and CDC replication scenarios in SQL Server.

@github-actions github-actions Bot added type/fix Type: Bug fix. Only for pull requests. ci/run-e2e-cdc-source-tests labels Sep 18, 2026
@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: d9bc6335-d502-44c1-9e40-c3eb5cedf6a7

📥 Commits

Reviewing files that changed from the base of the PR and between c4a578b and 2e1e1a5.

📒 Files selected for processing (3)
  • e2e_test/source_inline/cdc/sql_server/sql_server_cdc.slt.serial
  • e2e_test/source_inline/cdc/sql_server/sql_server_cdc_prepare.sql
  • src/connector/src/source/cdc/external/sql_server.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

SQL Server CDC metadata queries now preserve table and primary-key order. End-to-end tests add a composite-key source table and verify its mapped schema and replicated rows.

Changes

SQL Server CDC ordering

Layer / File(s) Summary
Preserve SQL Server metadata ordering
src/connector/src/source/cdc/external/sql_server.rs
The column metadata query orders results by ORDINAL_POSITION. The primary-key query orders results by kcu.ORDINAL_POSITION.
Validate composite-key CDC
e2e_test/source_inline/cdc/sql_server/sql_server_cdc_prepare.sql, e2e_test/source_inline/cdc/sql_server/sql_server_cdc.slt.serial
The test setup creates a CDC-enabled table with a three-column primary key and seed rows. The test verifies mapped column order, primary-key order, distribution-key order, and replicated rows.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 2e1e1

The change preserves SQL Server CDC column and composite-key ordering and adds regression coverage, with no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preserving SQL Server composite primary key order in the connector.
Description check ✅ Passed The description explains the fix, implementation approach, regression coverage, and related issue. It follows the required template and completes the relevant checklist items. The template requests a …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.98.0)

Clippy execution failed


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chenzl25 chenzl25 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@zwang28
zwang28 added this pull request to the merge queue Sep 18, 2026
Merged via the queue into main with commit ee8b82b Sep 18, 2026
71 of 72 checks passed
@zwang28
zwang28 deleted the check-sqlserver-cdc-issue branch September 18, 2026 09:33
@github-actions

Copy link
Copy Markdown
Contributor

✅ Cherry-pick PRs (or issues if encountered conflicts) have been created successfully to all target branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants