----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2831/#review3281 -----------------------------------------------------------
Looks good overall, but I think we should continue to require that the user specify the TEMPORARY keyword until we have a security mechanism in place to restrict access to non-temporary functions. ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java <https://reviews.apache.org/r/2831/#comment7343> Please don't move blocks of code around. It makes the diffs much harder to review. Please save cleanup/reorganization changes like this for refactoring patches. ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g <https://reviews.apache.org/r/2831/#comment7344> Since we don't yet have the security mechanism in place to handle global non-temporary functions, I think we should leave the grammar unchanged and continue to require that the user specify the "TEMPORARY" keyword. Please back out this change, along with the modifications to CreateFuncDesc and DropFuncDesc. ql/src/java/org/apache/hadoop/hive/ql/plan/CreateFunctionDesc.java <https://reviews.apache.org/r/2831/#comment7345> Please prefix boolean variable names with "is", e.g. "isTemporary". - Carl On 2011-11-15 07:06:04, Navis Ryu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2831/ > ----------------------------------------------------------- > > (Updated 2011-11-15 07:06:04) > > > Review request for hive and Carl Steinbach. > > > Summary > ------- > > Extension from HIVE-2503 for function registry. > > > This addresses bug HIVE-2573. > https://issues.apache.org/jira/browse/HIVE-2573 > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 4d4fce2 > ql/src/java/org/apache/hadoop/hive/ql/plan/CreateFunctionDesc.java 051095a > ql/src/java/org/apache/hadoop/hive/ql/plan/DropFunctionDesc.java 8a78f5b > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 07a4832 > ql/src/test/org/apache/hadoop/hive/ql/QTestUtil.java 35d124b > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 197bc77 > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionTask.java 70b6f57 > ql/src/java/org/apache/hadoop/hive/ql/parse/FunctionSemanticAnalyzer.java > f2b7018 > > Diff: https://reviews.apache.org/r/2831/diff > > > Testing > ------- > > > Thanks, > > Navis > >
