-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1324/#review1450
-----------------------------------------------------------


* The patch does not compile. Looks like it's missing the definition for the 
Substitution class. Why is this new class necessary? Why not build any new 
functionality into the VariableSubstitution class?
* This patch needs to add new testcases for the namespace prefixing.
* Why is the new namespace named "define"? If not "hivevar", can we at least 
use a noun instead of a verb?

- Carl


On 2011-08-08 22:44:45, Vaibhav Aggarwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1324/
> -----------------------------------------------------------
> 
> (Updated 2011-08-08 22:44:45)
> 
> 
> Review request for hive and Carl Steinbach.
> 
> 
> Summary
> -------
> 
> Create a separate namespace for Hive variables.
> 
> Added support for:
> 
> 1. -d and --define;
> 2. set define:var=var_value; // To set the variable
> 3. set -v;
> 4. set define:var; // To print the variable
> 
> Thanks
> Vaibhav
> 
> 
> This addresses bug HIVE-2020.
>     https://issues.apache.org/jira/browse/HIVE-2020
> 
> 
> Diffs
> -----
> 
>   cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java a2976b5 
>   cli/src/java/org/apache/hadoop/hive/cli/OptionsProcessor.java 90084ed 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java 
> e203dda 
>   ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java 97fa1ab 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 6a6e8e8 
> 
> Diff: https://reviews.apache.org/r/1324/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vaibhav
> 
>

Reply via email to