kasakrisz commented on code in PR #4330:
URL: https://github.com/apache/hive/pull/4330#discussion_r1230385242


##########
hplsql/src/main/java/org/apache/hive/hplsql/udf/Udf.java:
##########
@@ -37,77 +33,59 @@
 import org.apache.hive.hplsql.Scope;
 import org.apache.hive.hplsql.Var;
 import org.apache.hive.hplsql.executor.QueryExecutor;
-import org.apache.thrift.TException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-@Description(name = "hplsql", value = "_FUNC_('query' [, :1, :2, ...n]) - 
Execute HPL/SQL query", extended = "Example:\n" + " > SELECT 
_FUNC_('CURRENT_DATE') FROM src LIMIT 1;\n")
+@Description(name = "hplsql", value = "_FUNC_('query' [, :1, :2, ...n], 
'storedProcedure') - Execute HPL/SQL query",
+        extended = "Example:\n" + " > SELECT _FUNC_('CURRENT_DATE') FROM src 
LIMIT 1;\n")
 @UDFType(deterministic = false)
 public class Udf extends GenericUDF {
   public static String NAME = "hplsql";
-  transient Exec exec;
-  StringObjectInspector queryOI;
-  ObjectInspector[] argumentsOI;
-  private String functionDefinition;
+  private final transient Exec exec = new Exec();
+  private StringObjectInspector queryOI;
+  private ObjectInspector[] argumentsOI;
+  private StringObjectInspector funcDefOI;
+  private String functionDefinition = null;
+  private static final Logger LOG = 
LoggerFactory.getLogger(Udf.class.getName());
 
   public Udf() {
-  }
-
-  public Udf(Exec exec) {
-    this.exec = exec;
+    exec.setQueryExecutor(QueryExecutor.DISABLED);
+    exec.init();
   }
 
   /**
    * Initialize UDF
    */
   @Override
   public ObjectInspector initialize(ObjectInspector[] arguments) throws 
UDFArgumentException {
-    if (arguments.length == 0) {
-      throw new UDFArgumentLengthException("At least one argument must be 
specified");
+    if (arguments.length < 2) {
+      throw new UDFArgumentLengthException("At least two arguments must be 
specified");
     }
     if (!(arguments[0] instanceof StringObjectInspector)) {
       throw new UDFArgumentException("First argument must be a string");
     }
-    SessionState sessionState = SessionState.get();
-    if (sessionState != null) {
-      // we are still in HiveServer, get the source of the HplSQL function and 
store it.
-      functionDefinition = loadSource(sessionState, 
functionName(arguments[0]));
+    if (!(arguments[arguments.length-1] instanceof StringObjectInspector)) {
+      throw new UDFArgumentException("Last argument (stored procedure) must be 
a string");
     }
     queryOI = (StringObjectInspector)arguments[0];
+    funcDefOI = (StringObjectInspector)arguments[arguments.length-1];
     argumentsOI = arguments;
     return PrimitiveObjectInspectorFactory.javaStringObjectInspector;
   }
 
-  protected String loadSource(SessionState sessionState, String functionName) 
throws UDFArgumentException {
-    Exec exec = sessionState.getDynamicVar(Exec.class);
-    try {
-      StoredProcedure storedProcedure = exec.getMsc().getStoredProcedure(
-              new StoredProcedureRequest(
-                      SessionState.get().getCurrentCatalog(),
-                      SessionState.get().getCurrentDatabase(),
-                      functionName));
-      return storedProcedure != null ? storedProcedure.getSource() : null;
-    } catch (TException e) {
-      throw new UDFArgumentException(e);
-    }
-  }
-
-  protected String functionName(ObjectInspector argument) {
-    ConstantObjectInspector inspector = (ConstantObjectInspector) (argument);
-    String functionCall = inspector.getWritableConstantValue().toString();
-    return functionCall.split("\\(")[0].toUpperCase();
-  }
-
   /**
    * Execute UDF
    */
   @Override
   public Object evaluate(DeferredObject[] arguments) throws HiveException {
-    if (exec == null) {
-      exec = new Exec();
-      exec.setQueryExecutor(QueryExecutor.DISABLED);
-      exec.init();
-      if (functionDefinition != null) { // if it's null, it can be a built-in 
function
-        exec.parseAndEval(Arguments.script(functionDefinition));
+    if (functionDefinition == null) { // if it's null, it can be a built-in 
function
+      int idx = arguments.length-1;
+      setParameterForPrimitiveTypeArgument(":" + idx, arguments[idx].get(), 
funcDefOI);
+      functionDefinition = 
funcDefOI.getPrimitiveJavaObject(arguments[idx].get());
+      if (LOG.isDebugEnabled()) {

Review Comment:
   This check is unnecessary. Please remove it.



##########
hplsql/src/main/java/org/apache/hive/hplsql/udf/Udf.java:
##########
@@ -37,77 +33,59 @@
 import org.apache.hive.hplsql.Scope;
 import org.apache.hive.hplsql.Var;
 import org.apache.hive.hplsql.executor.QueryExecutor;
-import org.apache.thrift.TException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-@Description(name = "hplsql", value = "_FUNC_('query' [, :1, :2, ...n]) - 
Execute HPL/SQL query", extended = "Example:\n" + " > SELECT 
_FUNC_('CURRENT_DATE') FROM src LIMIT 1;\n")
+@Description(name = "hplsql", value = "_FUNC_('query' [, :1, :2, ...n], 
'storedProcedure') - Execute HPL/SQL query",
+        extended = "Example:\n" + " > SELECT _FUNC_('CURRENT_DATE') FROM src 
LIMIT 1;\n")
 @UDFType(deterministic = false)
 public class Udf extends GenericUDF {
   public static String NAME = "hplsql";
-  transient Exec exec;
-  StringObjectInspector queryOI;
-  ObjectInspector[] argumentsOI;
-  private String functionDefinition;
+  private final transient Exec exec = new Exec();
+  private StringObjectInspector queryOI;
+  private ObjectInspector[] argumentsOI;
+  private StringObjectInspector funcDefOI;
+  private String functionDefinition = null;
+  private static final Logger LOG = 
LoggerFactory.getLogger(Udf.class.getName());

Review Comment:
   nit.: static field declarations especially logger is usually the first in 
the class.



##########
hplsql/src/main/java/org/apache/hive/hplsql/udf/Udf.java:
##########
@@ -121,11 +99,20 @@ public Object evaluate(DeferredObject[] arguments) throws 
HiveException {
     }
   }
 
+  /**
+   * Getter for Exec object
+   *
+   * @return Exec
+   */
+  public Exec getExec() {
+    return this.exec;
+  }
+
   /**
    * Set parameters for the current call
    */
   void setParameters(DeferredObject[] arguments) throws HiveException {
-    for (int i = 1; i < arguments.length; i++) {
+    for (int i = 1; i < arguments.length-1; i++) {

Review Comment:
   nit.:
   ```
   arguments.length - 1
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to