CNDB-16182: CNDB-15608 simplify ceiling row id from primary key (#2142) #2180
Conversation
While working on CNDB-15608, IntelliJ lint complains were noticed, which are not related to the actual changes in the patch port. Thus I fix them in this separate commit to avoid unnecessary noise while working on the actual patch port. Many of the changes are align what I have already merged earlier in other PRs. Some of the changes might not match preferences from others and I am open for discussion. The changes include: - Remove unused imports - Use the formatter of the logger instead of string concatenation - Use method instead of lambda - Remove unnecessary suppression of resource warnings - Simplify Boolean conditions - Remove unnecessary modifiers in interfaces - Fix typos - Fixing links in javadoc comments - Add static modifier to nested classes - Remove class fields when not used - Remove unnecessary throws in method signatures - Use final when recommended - Remove unused method arguments - Replace single char strings with chars - Remove unnecessary null variable initialization - Replace assert true with assert equal - Change order of assert arguments to have expected value first - Remove unnecessary explicit casting
PartitionAwarePrimaryKeyMap implements overcomplicated `ceiling` method calling `exactRowIdOrInvertedCeiling`. This commit Simplifies PartitionAwarePrimaryKeyMap.ceiling to use the corresponding correct method from the reader directly. This can be seen as a follow up to https://github.com/datastax/cassandra/pull/1096/files#diff-c5011580ab9b0d99d9e504570c4cccb152221d3dbe62c8a956e83fce9070b380
Checklist before you submit for review
|
michaelsembwever
left a comment
michaelsembwever
left a comment
There was a problem hiding this comment.
discussion ongoing about the need for refactoring and cleaning in main-5.0.
my suggestion is the first commit is contributed (only) upstream, and we minimise this PR to the smallest changeset needed. (the follows the same practice for patches as upstream and its non-trunk branches.)
That's a fair point. To be effective going forward, as a wider team I guess we'll need to rely on PR reviewers pushing back on these types of changes on main-5.0 and asking authors to open separate tickets for upstream unless the changes are only relevant to our fork. |
This reverts commit 9e6201b.
djatnieks
changed the title
|
Yes agree. Let's first get agreement and aligned messaging within the team – as you point out there's a few legit questions and possible grey areas that will come up. |
There was a problem hiding this comment.
Will you (can you please) squash the three commits before merge ?
And make a mention in the commit message under Rebase notes: that sha 48b55fb was dropped, why, and that it should be upstreamed instead.
Is this ok ?
https://github.com/riptano/cndb/issues/16182
Port into main-5.0 commit 42ae0f3
CNDB-15608: simplify ceiling row id from primary key
PartitionAwarePrimaryKeyMap implements overcomplicated
ceilingmethodcalling
exactRowIdOrInvertedCeiling.This commit Simplifies PartitionAwarePrimaryKeyMap.ceiling to use the corresponding
correct method from the reader directly.
This can be seen as a follow up to
https://github.com/datastax/cassandra/pull/1096/files#diff-c5011580ab9b0d99d9e504570c4cccb152221d3dbe62c8a956e83fce9070b380