----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53059/#review153516 -----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenTimeRangeWriter.java (lines 47 - 48) <https://reviews.apache.org/r/53059/#comment222910> Let's make it more generic by asking for bound type on both ends of the range. values can be open and closed. parsing can be done by `com.google.common.collect.BoundType.valueOf(value.toUpperCase())`. In case start is OPEN, `start = start.previous()` In case end is OPEN, `end = end.next()` `next` and `previous` to be defined as per the next comment Default values for both will be `CLOSED`. lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenTimeRangeWriter.java (lines 84 - 98) <https://reviews.apache.org/r/53059/#comment222909> Cleaner way would be the following: ``` start = start.previous() end = end.next() ``` And define `previous` and `next` functions in `FactParition` ``` next(): return new FactPartition(getPartCol(), getTimePartition().next(), getContainingPart(), getStorageTables()) previous(): return new FactPartition(getPartCol(), getTimePartition().previous(), getContainingPart(), getStorageTables()) ``` lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/DruidSQLRewriter.java (lines 112 - 113) <https://reviews.apache.org/r/53059/#comment222911> When `having` and `orderby` are properly supported by plyql, we'll need to change the signatures back to this form. So, let's not change the signature, just pass `null` for now. lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/DruidSQLRewriter.java (lines 223 - 231) <https://reviews.apache.org/r/53059/#comment222914> Would ideally like to minimize the future change required when having and order by are officially supported by plyql community. It should be as simple as flipping a switch in the configuration. So we can have two properties denoting whether having is supported and orderby is supported. Please modify the changes to take this into account. Writing test cases should be helpful to check the working. The same query will rewritten in four differnt ways depending on values of those properties. lens-driver-jdbc/src/test/resources/drivers/jdbc/druid/jdbcdriver-site.xml (line 68) <https://reviews.apache.org/r/53059/#comment222915> default should be false. Please write test cases for the changes - Rajat Khandelwal On Oct. 20, 2016, 7:37 p.m., Rajitha R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53059/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2016, 7:37 p.m.) > > > Review request for lens and Rajat Khandelwal. > > > Bugs: LENS-1350 > https://issues.apache.org/jira/browse/LENS-1350 > > > Repository: lens > > > Description > ------- > > Fix for bug : Lens-1350 > > > Diffs > ----- > > > lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenTimeRangeWriter.java > 046149b > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java > f20f105 > > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/DruidSQLRewriter.java > eb1d69c > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > 82e0231 > > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriverConfConstants.java > f4e0451 > lens-driver-jdbc/src/test/resources/drivers/jdbc/druid/jdbcdriver-site.xml > e4fad23 > > Diff: https://reviews.apache.org/r/53059/diff/ > > > Testing > ------- > > > Thanks, > > Rajitha R > >
