Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Drill, mail # dev - Review Request 18841: Patch for DRILL-88


Copy link to this message
-
Re: Review Request 18841: Patch for DRILL-88
Aditya Kishore 2014-03-19, 10:36

This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18841/#review37600
Doing first pass while trying to understand the flow. Looks good so far with few comments.
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseGroupScan.java
<https://reviews.apache.org/r/18841/#comment69336>

    nit: There are many unused imports in some source files.

contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
<https://reviews.apache.org/r/18841/#comment69344>

    Set the scanner caching to a non-default (1) value. Probably equal to record batch size?

contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
<https://reviews.apache.org/r/18841/#comment69340>

    Throw a (subclass of) DrillRuntimeException?

contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
<https://reviews.apache.org/r/18841/#comment69338>

    result.raw() has been deprecated in 0.96 and later.

contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
<https://reviews.apache.org/r/18841/#comment69339>

    result.getBytes() does not exist in 0.96 and later.

contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
<https://reviews.apache.org/r/18841/#comment69343>

    Use org.apache.hadoop.hbase.util.Bytes.compareTo(byte[], int, int, byte[], int, int) instead. It uses faster Unsafe based comparer.

contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
<https://reviews.apache.org/r/18841/#comment69341>

    Did you mean setNull(i)?

distribution/src/resources/drill-config.sh
<https://reviews.apache.org/r/18841/#comment69214>

    Include HBase conf folder at the beginning of HBASE_CLASSPATH.
- Aditya Kishore
On March 6, 2014, 1:24 a.m., Steven Phillips wrote: