----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50740/#review144589 -----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenOnlyTimeRangeWriter.java (lines 38 - 79) <https://reviews.apache.org/r/50740/#comment210618> There is code duplication from `BetweenTimeRangeWriter`. Suggestions: 1. Polymorphism. One of these would extend the other. 2. Configurability. Keep only one class and Make use of `org.apache.hadoop.conf.Configurable` to set configuration whether to always go for between or not. lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java (line 926) <https://reviews.apache.org/r/50740/#comment210622> The name `Mode` suggests it to be an `enum`. Secondly, the append modes declared below have no state. The methods can as well be static there. There is no restriction wrt creating a bunch of instances of a LowerCaseAppendMode, while knowing that each instance does the exact same thing. So my suggestion would be to change it to the following: ```enum AppendMode { LOWER_CASE { @Override public String convert(String s) { return s.toLowerCase(); } }, DEFAULT; public String convert(String s) { return s; } } ``` lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java (line 938) <https://reviews.apache.org/r/50740/#comment210621> 1. Nochange => NoChange - Rajat Khandelwal On Aug. 3, 2016, 12:58 p.m., Rajitha R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50740/ > ----------------------------------------------------------- > > (Updated Aug. 3, 2016, 12:58 p.m.) > > > Review request for lens and Amareshwari Sriramadasu. > > > Bugs: LENS-1224 > https://issues.apache.org/jira/browse/LENS-1224 > > > Repository: lens > > > Description > ------- > > Changes done include : > 1. Druid sql rewriter in Jdbc driver > 2. New Timerangerewriter for Druid > 3. HQLParser toString modified > > > Diffs > ----- > > > lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenOnlyTimeRangeWriter.java > PRE-CREATION > lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 68cdcef > > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/DruidSQLRewriter.java > PRE-CREATION > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestDruidSQLRewriter.java > PRE-CREATION > lens-driver-jdbc/src/test/resources/drivers/jdbc/druid/jdbcdriver-site.xml > PRE-CREATION > > Diff: https://reviews.apache.org/r/50740/diff/ > > > Testing > ------- > > > Thanks, > > Rajitha R > >
