You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by sa...@yahoo.com on 2011/08/10 00:40:56 UTC
Review Request: HIVE-1916: Change Default Alias For Aggregated Columns (_c1)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1441/
-----------------------------------------------------------
Review request for hive and Ning Zhang.
Summary
-------
Default behavior will be as before.
Adding new Hive conf vars to make the column names include the aggregation function and params.
This addresses bug HIVE-1916.
https://issues.apache.org/jira/browse/HIVE-1916
Diffs
-----
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1155181
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1155181
trunk/ql/src/test/queries/clientpositive/autogen_colname.q PRE-CREATION
trunk/ql/src/test/results/clientpositive/autogen_colname.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/1441/diff
Testing
-------
Added new query file with expected results. All unit tests pass
Thanks,
sameerm
Re: Review Request: HIVE-1916: Change Default Alias For Aggregated Columns
(_c1)
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1441/#review1368
-----------------------------------------------------------
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/1441/#comment3136>
Please add these definitions to hive-default.xml along with a description for each property.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/1441/#comment3140>
These are initialized in the constructor. Please don't also initialize them here.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/1441/#comment3137>
Please run checkstyle and correct any violations introduced by your changes.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/1441/#comment3142>
Create a static final variable for the 20 char length limit.
- Carl
On 2011-08-09 22:40:56, sameerm wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1441/
> -----------------------------------------------------------
>
> (Updated 2011-08-09 22:40:56)
>
>
> Review request for hive and Ning Zhang.
>
>
> Summary
> -------
>
> Default behavior will be as before.
> Adding new Hive conf vars to make the column names include the aggregation function and params.
>
>
> This addresses bug HIVE-1916.
> https://issues.apache.org/jira/browse/HIVE-1916
>
>
> Diffs
> -----
>
> trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1155181
> trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1155181
> trunk/ql/src/test/queries/clientpositive/autogen_colname.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/autogen_colname.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/1441/diff
>
>
> Testing
> -------
>
> Added new query file with expected results. All unit tests pass
>
>
> Thanks,
>
> sameerm
>
>
Re: Review Request: HIVE-1916: Change Default Alias For Aggregated Columns
(_c1)
Posted by Ning Zhang <nz...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1441/#review1424
-----------------------------------------------------------
trunk/conf/hive-default.xml
<https://reviews.apache.org/r/1441/#comment3286>
change "aggregate function" to "function" to avoid confusion.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/1441/#comment3288>
styling nitpick: space between '='
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/1441/#comment3287>
remove the trailing space.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/1441/#comment3289>
remove trailing space
trunk/ql/src/test/queries/clientpositive/autogen_colname.q
<https://reviews.apache.org/r/1441/#comment3290>
in this test case can you add more functions such as '%', '/' and even '\n'? I think the parser should already removed '\n' or TABs but need to confirm in the unit test.
- Ning
On 2011-08-10 00:56:44, sameerm wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1441/
> -----------------------------------------------------------
>
> (Updated 2011-08-10 00:56:44)
>
>
> Review request for hive and Ning Zhang.
>
>
> Summary
> -------
>
> Default behavior will be as before.
> Adding new Hive conf vars to make the column names include the aggregation function and params.
>
>
> This addresses bug HIVE-1916.
> https://issues.apache.org/jira/browse/HIVE-1916
>
>
> Diffs
> -----
>
> trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1155181
> trunk/conf/hive-default.xml 1155181
> trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1155181
> trunk/ql/src/test/queries/clientpositive/autogen_colname.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/autogen_colname.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/1441/diff
>
>
> Testing
> -------
>
> Added new query file with expected results. All unit tests pass
>
>
> Thanks,
>
> sameerm
>
>
Re: Review Request: HIVE-1916: Change Default Alias For Aggregated Columns
(_c1)
Posted by Ning Zhang <nz...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1441/#review1464
-----------------------------------------------------------
Ship it!
LGTM
- Ning
On 2011-08-15 20:09:08, sameerm wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1441/
> -----------------------------------------------------------
>
> (Updated 2011-08-15 20:09:08)
>
>
> Review request for hive and Ning Zhang.
>
>
> Summary
> -------
>
> Default behavior will be as before.
> Adding new Hive conf vars to make the column names include the aggregation function and params.
>
>
> This addresses bug HIVE-1916.
> https://issues.apache.org/jira/browse/HIVE-1916
>
>
> Diffs
> -----
>
> trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1157970
> trunk/conf/hive-default.xml 1157970
> trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1157970
> trunk/ql/src/test/queries/clientpositive/autogen_colalias.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/autogen_colalias.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/1441/diff
>
>
> Testing
> -------
>
> Added new query file with expected results. All unit tests pass
>
>
> Thanks,
>
> sameerm
>
>
Re: Review Request: HIVE-1916: Change Default Alias For Aggregated Columns
(_c1)
Posted by sa...@fb.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1441/
-----------------------------------------------------------
(Updated 2011-08-15 20:09:08.444993)
Review request for hive and Ning Zhang.
Changes
-------
Using "columnalias" instead of "columnname" to be consistent with description and code intent. e.g. Hive params are now called "hive.autogen.columnalias.*" instead of "hive.autogen.columnname."
Fixing style issues.
Updated unit test query script with more select expressions including newlines and tabs.
Summary
-------
Default behavior will be as before.
Adding new Hive conf vars to make the column names include the aggregation function and params.
This addresses bug HIVE-1916.
https://issues.apache.org/jira/browse/HIVE-1916
Diffs (updated)
-----
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1157970
trunk/conf/hive-default.xml 1157970
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1157970
trunk/ql/src/test/queries/clientpositive/autogen_colalias.q PRE-CREATION
trunk/ql/src/test/results/clientpositive/autogen_colalias.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/1441/diff
Testing
-------
Added new query file with expected results. All unit tests pass
Thanks,
sameerm
Re: Review Request: HIVE-1916: Change Default Alias For Aggregated Columns
(_c1)
Posted by sa...@yahoo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1441/
-----------------------------------------------------------
(Updated 2011-08-10 00:56:44.290697)
Review request for hive and Ning Zhang.
Changes
-------
Added hive.autogen.columnname.prefix.label and hive.autogen.columnname.prefix.includefuncname to hive-default.xml
Created a static final variable for the 20 char length limit in SemanticAnalyzer.java
Took out default variable values from SemanticAnalyzer.java (will be set in constructor)
Fixed checkstyle line length warnings introduced by patch in HiveConf.java, SemanticAnalyzer.java
Added some more comments in SemanticAnalyzer.java
Summary
-------
Default behavior will be as before.
Adding new Hive conf vars to make the column names include the aggregation function and params.
This addresses bug HIVE-1916.
https://issues.apache.org/jira/browse/HIVE-1916
Diffs (updated)
-----
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1155181
trunk/conf/hive-default.xml 1155181
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1155181
trunk/ql/src/test/queries/clientpositive/autogen_colname.q PRE-CREATION
trunk/ql/src/test/results/clientpositive/autogen_colname.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/1441/diff
Testing
-------
Added new query file with expected results. All unit tests pass
Thanks,
sameerm