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




ql/src/java/org/apache/hadoop/hive/ql/ddl/function/ShowFunctionsOperation.java
Lines 57 (patched)
<https://reviews.apache.org/r/70415/#comment300654>

    please remove this log; and add the "pattern" to the other "info" level 
one...



ql/src/java/org/apache/hadoop/hive/ql/ddl/function/ShowFunctionsOperation.java
Lines 58 (patched)
<https://reviews.apache.org/r/70415/#comment300653>

    This feature seems to be missing some documentation...
    https://cwiki.apache.org/confluence/display/Hive/LanguageManual+UDF
    
    I don't fully understand what is deprecated at this point; it has a 
"pattern" but it's not a "is-like pattern?"
    apparently `SHOW FUNCTION '.*x'` is deprecated; but `SHOW FUNCTIONS LIKE 
'.*x'` is accepted
    
    if it's unsupported: then why don't we return a parse error from 
analyzeShowFunctions at compile time?
    
    I know you are just moving this method...



ql/src/java/org/apache/hadoop/hive/ql/plan/DescFunctionDesc.java
Line 46 (original), 41 (patched)
<https://reviews.apache.org/r/70415/#comment300652>

    this comment is consusing...
    schema is usually the resulting schema of the task - I don't think it has 
anything to do with thrift
    
    please remove it


- Zoltan Haindrich


On April 6, 2019, 12:51 p.m., Miklos Gergely wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70415/
> -----------------------------------------------------------
> 
> (Updated April 6, 2019, 12:51 p.m.)
> 
> 
> Review request for hive and Zoltan Haindrich.
> 
> 
> Bugs: HIVE-21567
>     https://issues.apache.org/jira/browse/HIVE-21567
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> DDLTask is a huge class, more than 5000 lines long. The related DDLWork is 
> also a huge class, which has a field for each DDL operation it supports. The 
> goal is to refactor these in order to have everything cut into more 
> handleable classes under the package  org.apache.hadoop.hive.ql.exec.ddl:
> 
> have a separate class for each operation
> have a package for each operation group (database ddl, table ddl, etc), so 
> the amount of classes under a package is more manageable
> make all the requests (DDLDesc subclasses) immutable
> DDLTask should be agnostic to the actual operations
> right now let's ignore the issue of having some operations handled by DDLTask 
> which are not actual DDL operations (lock, unlock, desc...)
> In the interim time when there are two DDLTask and DDLWork classes in the 
> code base the new ones in the new package are called DDLTask2 and DDLWork2 
> thus avoiding the usage of fully qualified class names where both the old and 
> the new classes are in use.
> 
> Step #4: extract all the function related operations from the old DDLTask, 
> and move them under the new package.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLOperationContext.java 
> 14744d1461 
>   ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLTask2.java 1f9a0bb173 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/function/DescFunctionOperation.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/ddl/function/ShowFunctionsOperation.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/ddl/function/package-info.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java ed797fc5dc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 0828fade5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 73acc31a27 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DescFunctionDesc.java 9898b1584d 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowFunctionsDesc.java 
> 609d1740a6 
>   ql/src/test/queries/clientpositive/desc_function.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/show_functions.q 106e05ff14 
>   ql/src/test/results/clientpositive/desc_function.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/show_functions.q.out 4e44753241 
> 
> 
> Diff: https://reviews.apache.org/r/70415/diff/1/
> 
> 
> Testing
> -------
> 
> All the q tests are still running.
> 
> 
> Thanks,
> 
> Miklos Gergely
> 
>

Reply via email to