You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Rajitha R <ra...@gmail.com> on 2016/08/03 07:24:48 UTC
Review Request 50740: Lens Druid sql rewriter changes
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/
-----------------------------------------------------------
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
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/#review147117
-----------------------------------------------------------
lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/DruidSQLRewriter.java (lines 98 - 99)
<https://reviews.apache.org/r/50740/#comment214214>
Default constructor need not be explicitly defined.
lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/DruidSQLRewriter.java (lines 102 - 111)
<https://reviews.apache.org/r/50740/#comment214215>
Duplication from ColumnarSQLRewriter. This code has been refactored to single line:
regexReplaceMap = CommonUtils.parseMapFromString(conf.get(JDBCDriverConfConstants.REGEX_REPLACEMENT_VALUES));
lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/DruidSQLRewriter.java (lines 171 - 185)
<https://reviews.apache.org/r/50740/#comment214216>
Can we avoid code duplication from ColumnarSQLRewriter?
- Rajat Khandelwal
On Aug. 25, 2016, 5:09 p.m., Rajitha R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50740/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2016, 5:09 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/BetweenTimeRangeWriter.java a5b26c4
> lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 49ed5ef
> lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 68cdcef
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestBetweenTimeRangeWriter.java eeba861
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriter.java 748f92f
> 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
>
>
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/#review148895
-----------------------------------------------------------
Ship it!
Ship It!
- Rajat Khandelwal
On Sept. 8, 2016, 5:51 p.m., Rajitha R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50740/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2016, 5:51 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. BetweenTimerangerewriter changes for Druid
> 3. HQLParser toString modified
>
>
> Diffs
> -----
>
> lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenTimeRangeWriter.java a5b26c4
> lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 49ed5ef
> lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 68cdcef
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestBetweenTimeRangeWriter.java eeba861
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriter.java 748f92f
> lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/ColumnarSQLRewriter.java b1fd459
> 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
>
>
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajitha R <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/
-----------------------------------------------------------
(Updated Sept. 8, 2016, 12:21 p.m.)
Review request for lens and Amareshwari Sriramadasu.
Changes
-------
Checkstyle fix
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. BetweenTimerangerewriter changes for Druid
3. HQLParser toString modified
Diffs (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenTimeRangeWriter.java a5b26c4
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 49ed5ef
lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 68cdcef
lens-cube/src/test/java/org/apache/lens/cube/parse/TestBetweenTimeRangeWriter.java eeba861
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriter.java 748f92f
lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/ColumnarSQLRewriter.java b1fd459
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
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajitha R <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/
-----------------------------------------------------------
(Updated Sept. 8, 2016, 11:33 a.m.)
Review request for lens and Amareshwari Sriramadasu.
Changes
-------
Changes post review comments
Bugs: LENS-1224
https://issues.apache.org/jira/browse/LENS-1224
Repository: lens
Description (updated)
-------
Changes done include :
1. Druid sql rewriter in Jdbc driver
2. BetweenTimerangerewriter changes for Druid
3. HQLParser toString modified
Diffs (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenTimeRangeWriter.java a5b26c4
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 49ed5ef
lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 68cdcef
lens-cube/src/test/java/org/apache/lens/cube/parse/TestBetweenTimeRangeWriter.java eeba861
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriter.java 748f92f
lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/ColumnarSQLRewriter.java b1fd459
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
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajitha R <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/
-----------------------------------------------------------
(Updated Aug. 25, 2016, 11:39 a.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 (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/parse/BetweenTimeRangeWriter.java a5b26c4
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 49ed5ef
lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 68cdcef
lens-cube/src/test/java/org/apache/lens/cube/parse/TestBetweenTimeRangeWriter.java eeba861
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriter.java 748f92f
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
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajitha R <ra...@gmail.com>.
> On Aug. 3, 2016, 7:41 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java, line 938
> > <https://reviews.apache.org/r/50740/diff/1/?file=1461103#file1461103line938>
> >
> > 1. Nochange => NoChange
Using enum as mentioned in earlier comment, this would be the default mode
- Rajitha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/#review144589
-----------------------------------------------------------
On Aug. 3, 2016, 7:28 a.m., Rajitha R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50740/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2016, 7:28 a.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
>
>
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 50740: Lens Druid sql rewriter changes
Posted by Rajitha R <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50740/
-----------------------------------------------------------
(Updated Aug. 3, 2016, 7:28 a.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 (updated)
-----
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