Skip to content

Conversation

@dybyte
Copy link
Contributor

@dybyte dybyte commented Dec 23, 2025

Fixes: #9949

Purpose of this pull request

This PR fixes the partitioning column selection logic.
It ensures that when a primary/unique key is present, the first column of that key is correctly selected and used as the partitioning column.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added JdbcSourceChunkSplitterTest.splitColumnTestWithUniqueKey().

Check list

@dybyte dybyte marked this pull request as draft December 23, 2025 13:25
@dybyte dybyte marked this pull request as ready for review December 23, 2025 13:34
davidzollo
davidzollo previously approved these changes Dec 24, 2025
Copy link
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

LGTM

@Carl-Zhou-CN
Copy link
Member

Carl-Zhou-CN commented Dec 25, 2025

@dybyte hi,Are all jdbc databases adapted to the leftmost principle?

@dybyte
Copy link
Contributor Author

dybyte commented Dec 25, 2025

@dybyte hi,Are all jdbc databases adapted to the leftmost principle?

Yes. The leftmost column of a composite index is simply the first column defined in the index.
However, I’m not entirely sure if I’ve understood your question correctly.

Copy link
Member

@Carl-Zhou-CN Carl-Zhou-CN left a comment

Choose a reason for hiding this comment

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

LGTM

if (isEvenlySplitColumn(firstColumn)) {
splitColumn = columnComparable(splitColumn, firstColumn);
if (sqlTypePriority(splitColumn) == 1) {
return splitColumn;
Copy link
Member

Choose a reason for hiding this comment

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

How about printing a log after selecting the column, so that users can know some details.

import java.util.Optional;

public class JdbcSourceChunkSplitterTest {
class JdbcSourceChunkSplitterTest {
Copy link
Member

Choose a reason for hiding this comment

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

Why is "public" deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JUnit5 does not require test classes to be public, so the modifier was removed as it is unnecessary.

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

Projects

None yet

5 participants