----------------------------------------------------------- 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 > >