Support custom JdbcReadWithPartitionsHelper #31733
Conversation
|
R: @Abacn |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
| @@ -443,7 +442,7 @@ private static Calendar withTimestampAndTimezone(DateTime dateTime) { | |||
| * | |||
| * @param <PartitionT> | |||
| */ | |||
| interface JdbcReadWithPartitionsHelper<PartitionT> | |||
| public interface JdbcReadWithPartitionsHelper<PartitionT> | |||
There was a problem hiding this comment.
JdbcUtil is still package private. One would not be able to instantiate JdbcReadWithPartitionsHelper outside the same package.
Amar3tto
force-pushed
the
jdbc-read-with-partitions-helper
branch
from
df0a896
to
094dc70
Compare
| public Iterable<KV<String, String>> calculateRanges( | ||
| String lowerBoundStr, String upperBoundStr, Long partitions) { | ||
| List<KV<String, String>> ranges = new ArrayList<>(); | ||
| long lowerBound = Long.parseLong(lowerBoundStr); |
There was a problem hiding this comment.
this is a bad partition logic. I was doing this also in #31702 at the beginning. However it turns out that this does not read all rows. suppose we have "0" to "999", and partition into 2 strips, it will become "0" - "500"; "501" to "1000", however "501" > "1000", and "9" > "500" in string comparison.
If go with this PR, could you please integrate the test added in #31702. It can also be an example of custom string type partition helper.
Amar3tto
force-pushed
the
jdbc-read-with-partitions-helper
branch
from
094dc70
to
f10e29b
Compare
Fixes #27120
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.