mdayakar commented on code in PR #5084:
URL: https://github.com/apache/hive/pull/5084#discussion_r1502679370
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java:
##########
@@ -132,39 +133,84 @@ private boolean execProc(String name,
HplsqlParser.Expr_func_paramsContext ctx)
/**
* Set parameters for user-defined function call
*/
- public static void setCallParameters(String procName,
HplsqlParser.Expr_func_paramsContext actual, ArrayList<Var> actualValues,
- HplsqlParser.Create_routine_paramsContext formal,
- HashMap<String, Var> out,
- Exec exec) {
- if (actual == null || actual.func_param() == null || actualValues == null)
{
+ public static void setCallParameters(String procName,
HplsqlParser.Expr_func_paramsContext actual,
+ ArrayList<Var> actualValues, HplsqlParser.Create_routine_paramsContext
formal, HashMap<String, Var> out,
+ Exec exec) {
+ // if it is a non-parameter function then just return.
+ if (actual == null && formal == null) {
return;
}
- int actualCnt = actualValues.size();
- int formalCnt = formal.create_routine_param_item().size();
- if (formalCnt != actualCnt) {
- throw new ArityException(actual.getParent(), procName, formalCnt,
actualCnt);
+ int actualCnt = (actualValues == null) ? 0 : actualValues.size();
+ int passedParamCnt = actualCnt;
+ List<HplsqlParser.Create_routine_param_itemContext> routineParamItem =
formal.create_routine_param_item();
+ int formalCnt = routineParamItem.size();
+ ParserRuleContext ruleContext = (actual == null) ? null :
actual.getParent();
+ if (actualCnt > formalCnt) {
+ throw new ArityException(ruleContext, procName, formalCnt, actualCnt);
}
- for (int i = 0; i < actualCnt; i++) {
- HplsqlParser.ExprContext a = actual.func_param(i).expr();
- HplsqlParser.Create_routine_param_itemContext p =
getCallParameter(actual, formal, i);
- String name = p.ident().getText();
- String type = p.dtype().getText();
- String len = null;
- String scale = null;
- if (p.dtype_len() != null) {
- len = p.dtype_len().L_INT(0).getText();
- if (p.dtype_len().L_INT(1) != null) {
- scale = p.dtype_len().L_INT(1).getText();
- }
+ Map<String, Integer> defaultParamNamesVsIndexes = new HashMap<>();
+ if (actualCnt != formalCnt) {
+ populateDefaultParamDetails(routineParamItem, formalCnt,
defaultParamNamesVsIndexes);
+ actualCnt = actualCnt + defaultParamNamesVsIndexes.size();
+ if (actualCnt < formalCnt) {
+ throw new ArityException(ruleContext, procName, formalCnt,
passedParamCnt);
+ }
Review Comment:
> actualCnt can be higher then formalCnt. Let's say we have a function with
2 formal parameters both has default values. When calling the function we pass
only one actual parameter. The result is actualCnt = 1 + 2 = 3 which is
incorrect.
Here `actualCnt` contains the actual parameters passed + all default
parameters of the procedure. Which can be greater than `formalCnt` as user can
pass values for some default params along with non-default params. For the same
example a function with 2 formal parameters both has default values. When
calling the function we pass only one actual parameter then `actualCnt = 1 + 2
= 3` is correct why because user passed one parameter and for other parameter
we take default value and execute the procedure. Thats why here we are checking
only `actualCnt < formalCnt`.
> What is the purpose of this check here? if (actualCnt < formalCnt)
As explained above if user passes lesser parameters than the required
parameters then we should throw exception. For example if a function has 5
total params, in that lets say 2 default params, so here user has to pass
minimum 3 param values but if user passes only 2 then 2+2=4 which is lesser
than formal count so need to throw exception.
> The check in the end of the method should catch such cases too
if ((passedParamCnt + defaultParamNamesVsIndexes.size()) < formalCnt) {
This check is done after processing the values. This is useful if user
passes param values in named parameter binding. For example if a function takes
3 params in that 1 param is default one. Here user passes the values for one
non-default param and default param using named parameter binding then we
should throw exception as one non-default param value is not passed.
--
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: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
