[
https://issues.apache.org/jira/browse/HBASE-15333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15257618#comment-15257618
]
Jonathan Hsieh commented on HBASE-15333:
----------------------------------------
Here's my review on v5.
DefaultSource.scala
These replacements are fairly complicated -- more complicated that what they
replace. Add a comment to convert this into a verification exercise as opposed
to a reverse engineering exercise?
DefaultSource.scala:618: typo- "righRange"
DefaultSource.scala:617-627: Make a "boolean hasOverlap(left,right)" method to
make this easier to read?
DynamicLogicExpression.scala:28
- why are these encoding types in FilterOps? would i tmake sense to put them
in a separate class?
- why are we encoding these in naively using Bytes.xxxx? this is likely a
place we will want to have flexibility (e.g. we may want to encode data
differently). These encodings are not going to be any good for semantic
sort-ordered in row keys. This is specifically where the data encoding
mechanisms in hbase's OrderedBytes [1] that Nick D mentioned on the first
comment. Why not use that instead of coming up with yet another encoding
scheme?
DynamicLogicExpression.scala:200/297 Why is "Pass" changed to "dummy Pass -1"?
BoundRange.scala:26
can we use scaladoc formatting for comments where appropriate?
for bound range, are values inclusive or exclusive ranges?
DynamicLogicExrpressionSuite.scala
- FilterOps.encode just seems strange. Why isn't it something like
"JavaBytesEncoder.encode" (and allow for other encodings?)
PartitionFilterSuite.scala
- I need to run the code in order to verify it. Could we get the expected
'show' output string and then do a strcmp instead of just a count?
[1] See
https://github.com/apache/hbase/blob/master/hbase-common/src/main/java/org/apache/hadoop/hbase/util/OrderedBytes.java
> Enhance the filter to handle short, integer, long, float and double
> -------------------------------------------------------------------
>
> Key: HBASE-15333
> URL: https://issues.apache.org/jira/browse/HBASE-15333
> Project: HBase
> Issue Type: Sub-task
> Reporter: Zhan Zhang
> Assignee: Zhan Zhang
> Attachments: HBASE-15333-1.patch, HBASE-15333-2.patch,
> HBASE-15333-3.patch, HBASE-15333-4.patch, HBASE-15333-5.patch
>
>
> Currently, the range filter is based on the order of bytes. But for java
> primitive type, such as short, int, long, double, float, etc, their order is
> not consistent with their byte order, extra manipulation has to be in place
> to take care of them correctly.
> For example, for the integer range (-100, 100), the filter <= 1, the current
> filter will return 0 and 1, and the right return value should be (-100, 1]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)