> On April 28, 2014, 9:50 p.m., Jinfeng Ni wrote:
> > contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseFilterBuilder.java,
> > line 66
> > <https://reviews.apache.org/r/20806/diff/2/?file=570028#file570028line66>
> >
> > I feel we miss logic to check if a filter could be pushed into
> > HBaseScan. Such logic would be either in the rule HBasePushFilterIntoScan,
> > or before we call HBaseFilterBuilder.getHBaseScanSpec().
> >
> > For instance, if we have filter : row_key = 'value1' or row_key =
> > 'value2'. I assume this filter should not be pushed into HBase. However,
> > HBaseFilterBuilder.visitFunctionCall will be invoked for both row_key =
> > 'value' and row_key = 'value2'.
> >
> > Same issue could happen for filter : row_key = 'value' or other_column
> > = 'value2'.
> >
>
> Aditya Kishore wrote:
> Right now, we are not traversing the expression tree. So a condition like
> "row_key = 'value1' or row_key = 'value2'" will not be pushed down. However,
> eventually we would want to push it down as a a filter list of
> (RowKeyFilter(=, 'value1') OR RowKeyFilter(=, 'value2')).
>
> The second case, of course could never be pushed down.
>
> Jinfeng Ni wrote:
> What do you mean by "not traversing the expression tree"? I thought we
> use ExprVisitor, which will traverse the expr tree. so, for condition like
> "row_key = 'value1' or row_key = 'value2'", visitFunctionCall will be called
> three times : two for the = operators, one for "or" operator. Is that right?
> or I miss something here?
ExprVisitor will only traverse the tree if the implantation explicitly visit
all the children of the current node. That would mean something like
for(LogicalExpression child : e) {
child.accept(this, value));
}
in visitFunctionCall(). Without this it will just visit the top level
expression and return.
- Aditya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20806/#review41682
-----------------------------------------------------------
On April 29, 2014, 3:10 a.m., Aditya Kishore wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20806/
> -----------------------------------------------------------
>
> (Updated April 29, 2014, 3:10 a.m.)
>
>
> Review request for drill, Jinfeng Ni, Steven Phillips, and Timothy Chen.
>
>
> Bugs: DRILL-571
> https://issues.apache.org/jira/browse/DRILL-571
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch adds an Optimizer rule to HBase storage plugin which would
> transform an HBase scan followed by a logical (==, !=, <, <=, >, >=) filter
> on the row key into an HBase scan with filter thus minimizing the data
> transfer and elimination one operation altogether.
>
>
> Diffs
> -----
>
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/DrillHBaseConstants.java
> PRE-CREATION
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseFilterBuilder.java
> PRE-CREATION
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseGroupScan.java
> b8b6af4
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBasePushFilterIntoScan.java
> PRE-CREATION
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
> 8e1e0ac
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java
> 157d84a
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanSpec.java
> PRE-CREATION
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseSchemaFactory.java
> 991685c
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePlugin.java
> a82c6c3
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePluginConfig.java
> a5dfc35
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseSubScan.java
> 81a8af5
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseUtils.java
> PRE-CREATION
>
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HTableReadEntry.java
> e18b28d
>
> contrib/storage-hbase/src/test/java/org/apache/drill/hbase/HBaseRecordReaderTest.java
> 8a476c3
>
> contrib/storage-hbase/src/test/java/org/apache/drill/hbase/HBaseTestsSuite.java
> df6a98e
>
> contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestHBaseFilterPushDown.java
> PRE-CREATION
>
> contrib/storage-hbase/src/test/java/org/apache/drill/hbase/TestTableGenerator.java
> 2207f1d
>
> contrib/storage-hbase/src/test/resources/hbase/hbase_scan_screen_physical.json
> ae42d6b
>
> contrib/storage-hbase/src/test/resources/hbase/hbase_scan_screen_physical_column_select.json
> 7940c65
>
> contrib/storage-hbase/src/test/resources/hbase/hbase_scan_screen_physical_family_select.json
> 2dcc81c
> contrib/storage-hbase/src/test/resources/storage-plugins.json PRE-CREATION
> distribution/src/resources/storage-plugins.json 6f2c015
>
> Diff: https://reviews.apache.org/r/20806/diff/
>
>
> Testing
> -------
>
> Added a new test case TestHBaseFilterPushDown which is currently disabled
> since the Zookeeper port used by MiniHBase cluster is random and there was no
> clean way to pass this to the HBase storage plugin from SQL.
>
> To test it, start a local HBase cluster (./bin/start-hbase.sh) and un-comment
> the Junit's @Ignore annotation.
>
>
> Thanks,
>
> Aditya Kishore
>
>
