saintstack commented on a change in pull request #1062: HBASE-23705 Add
CellComparator to HFileContext
URL: https://github.com/apache/hbase/pull/1062#discussion_r368610567
##########
File path:
hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/RowIndexSeekerV1.java
##########
@@ -154,19 +153,6 @@ private int binarySearch(Cell seekCell, boolean
seekBefore) {
}
}
- private int compareRows(ByteBuffer row, Cell seekCell) {
Review comment:
We can't have encoders doing their own logic figuring Cell Compare else the
issue this patch fixes will perpetually haunt us as encoding implementors make
the same mistake of around comparators over and over again; i.e. presuming only
one comparator in the system or knowing there are two at least but defaulting
to user-space comparator since that is what is used 99.9% of the time.
CellComparator already has methods that do row compare. This is just adding
one that hosts the row in ByteBuffer instead of an array... an override.
CellComparators need to do more ByteBuffering, not less. CellComparator
needs to learn how to do compares w/o making copies of the ByteBuffer content
as it currently does in BBUtills if the BB doesn't have an array because
comparators don't know how to stride through a ByteBuffer.
bq. Also within this context we know the passed row BB is sliced for the rk
bytes alone. Within CellComparator we can not have such assumption.
CellComparator has one method that does this already taking an array.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
