From 8f839699afa7f1d65a3bcbbd283e4219580db948 Mon Sep 17 00:00:00 2001 From: mdayakar Date: Wed, 14 Feb 2024 10:54:23 +0530 Subject: [PATCH 1/7] HIVE-27490: HPL/SQL says it support default value for parameters but not considering them when no value is passed --- .../functions/InMemoryFunctionRegistry.java | 46 +++++++++++---- .../hive/beeline/TestHplSqlViaBeeLine.java | 56 +++++++++++++++++++ 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java index 262e8c74bc93..056eab81e970 100644 --- a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java +++ b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.antlr.v4.runtime.ParserRuleContext; @@ -136,28 +137,49 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par HplsqlParser.Create_routine_paramsContext formal, HashMap out, Exec exec) { - if (actual == null || actual.func_param() == null || actualValues == 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 routine_param_item_list = formal.create_routine_param_item(); + int formalCnt = routine_param_item_list.size(); + List defaultParamIndexes = new ArrayList<>(); + if (actualCnt != formalCnt) { + for (int i = 0; i < formalCnt; i++) { + if (routine_param_item_list.get(i).dtype_default() != null) { + defaultParamIndexes.add(i); + } + } + actualCnt = actualCnt + defaultParamIndexes.size(); + if (actualCnt < formalCnt) { + throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, passedParamCnt); + } } - 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); + for (int i = 0; i < formalCnt; i++) { + HplsqlParser.ExprContext a = null; + HplsqlParser.Create_routine_param_itemContext p = null; + Var value = null; + if (actual == null || actual.func_param() == null || actual.func_param(i) == null) { + if (!defaultParamIndexes.contains(i)) { + throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, passedParamCnt); + } + p = formal.create_routine_param_item().get(i); + a = p.dtype_default().expr(); + value = exec.evalPop(p.dtype_default().expr()); + } else { + a = actual.func_param(i).expr(); + p = getCallParameter(actual, formal, i); + value = actualValues.get(i); + } String name = p.ident().getText(); String type = p.dtype().getText(); String len = null; - String scale = 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(); } } - Var var = setCallParameter(name, type, len, scale, actualValues.get(i), exec); + Var var = setCallParameter(name, type, len, scale, value, exec); exec.trace(actual, "SET PARAM " + name + " = " + var.toString()); if (out != null && a.expr_atom() != null && a.expr_atom().qident() != null && (p.T_OUT() != null || p.T_INOUT() != null)) { diff --git a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java index eb2285ba16c5..d9a2c2df4e33 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java @@ -888,6 +888,62 @@ public void testTableAliasInColumnName() throws Throwable { testScriptFile(SCRIPT_TEXT, args(), "2023 = Hive"); } + @Test + public void testHplSqlProcedureCallingWithAllDefaultValues() throws Throwable { + String SCRIPT_TEXT = + "DROP TABLE IF EXISTS result;\n" + + "CREATE TABLE result (s string);\n" + + "CREATE PROCEDURE p1(s STRING DEFAULT 'default_val', num NUMBER DEFAULT 123)\n" + + "BEGIN\n" + + "INSERT INTO result VALUES(s || ' = ' || num);\n" + + "END;\n" + + "p1();\n" + + "SELECT * FROM result;" ; + testScriptFile(SCRIPT_TEXT, args(), "default_val = 123"); + } + + @Test + public void testHplSqlProcedureCallingWithSomeDefaultValues() throws Throwable { + String SCRIPT_TEXT = + "DROP TABLE IF EXISTS result;\n" + + "CREATE TABLE result (s string);\n" + + "CREATE PROCEDURE p1(s STRING DEFAULT 'default_val', num NUMBER DEFAULT 123)\n" + + "BEGIN\n" + + "INSERT INTO result VALUES(s || ' = ' || num);\n" + + "END;\n" + + "p1('Pass_Value');\n" + + "SELECT * FROM result;" ; + testScriptFile(SCRIPT_TEXT, args(), "Pass_Value = 123"); + } + + @Test + public void testHplSqlProcedureWithDefaultValues() throws Throwable { + String SCRIPT_TEXT = + "DROP TABLE IF EXISTS result;\n" + + "CREATE TABLE result (s string);\n" + + "CREATE PROCEDURE p1(s STRING DEFAULT 'default_val', num NUMBER)\n" + + "BEGIN\n" + + "INSERT INTO result VALUES(s || ' = ' || num);\n" + + "END;\n" + + "p1(111);\n" + + "SELECT * FROM result;" ; + testScriptFile(SCRIPT_TEXT, args(), ""); + } + + @Test + public void testHplSqlProcedureWithSomeDefaultValues() throws Throwable { + String SCRIPT_TEXT = + "DROP TABLE IF EXISTS result;\n" + + "CREATE TABLE result (s string);\n" + + "CREATE PROCEDURE p1(s STRING, num NUMBER DEFAULT 123)\n" + + "BEGIN\n" + + "INSERT INTO result VALUES(s || ' = ' || num);\n" + + "END;\n" + + "p1('Passed_Val');\n" + + "SELECT * FROM result;" ; + testScriptFile(SCRIPT_TEXT, args(), "Passed_Val = 123"); + } + private static List args() { return Arrays.asList("-d", BeeLine.BEELINE_DEFAULT_JDBC_DRIVER, "-u", miniHS2.getBaseJdbcURL() + ";mode=hplsql", "-n", userName); From 28a6cd8673f2cb8e93fab500856456d9095f04d4 Mon Sep 17 00:00:00 2001 From: mdayakar Date: Thu, 15 Feb 2024 20:57:35 +0530 Subject: [PATCH 2/7] Fixed test failures --- .../hive/hplsql/functions/InMemoryFunctionRegistry.java | 7 +++++++ .../java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java index 056eab81e970..1111ae635641 100644 --- a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java +++ b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java @@ -137,10 +137,17 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par HplsqlParser.Create_routine_paramsContext formal, HashMap out, Exec exec) { + // if it is a non-parameter function then just return. + if (actual == null && formal == null) { + return; + } int actualCnt = actualValues == null ? 0 : actualValues.size(); int passedParamCnt = actualCnt; List routine_param_item_list = formal.create_routine_param_item(); int formalCnt = routine_param_item_list.size(); + if (actualCnt > formalCnt) { + throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, actualCnt); + } List defaultParamIndexes = new ArrayList<>(); if (actualCnt != formalCnt) { for (int i = 0; i < formalCnt; i++) { diff --git a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java index d9a2c2df4e33..617b237615c5 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java @@ -107,7 +107,7 @@ public void testHplSqlProcedure() throws Throwable { "p1();\n" + "SELECT * FROM result;\n" + "/\n"; - testScriptFile(SCRIPT_TEXT, args(), "Hello world"); + testScriptFile(SCRIPT_TEXT, args(), "wrong number of arguments in call to 'p1'. Expected 1 got 0.", OutStream.ERR); } @Test @@ -927,7 +927,7 @@ public void testHplSqlProcedureWithDefaultValues() throws Throwable { "END;\n" + "p1(111);\n" + "SELECT * FROM result;" ; - testScriptFile(SCRIPT_TEXT, args(), ""); + testScriptFile(SCRIPT_TEXT, args(), "wrong number of arguments in call to 'p1'. Expected 2 got 1.", OutStream.ERR); } @Test From 54e197dd25b09177cdb35f866c407cb7580a3ab8 Mon Sep 17 00:00:00 2001 From: mdayakar Date: Thu, 15 Feb 2024 21:01:15 +0530 Subject: [PATCH 3/7] Fixed code comments --- .../hplsql/functions/InMemoryFunctionRegistry.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java index 1111ae635641..a30c68f658fd 100644 --- a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java +++ b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java @@ -143,15 +143,15 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par } int actualCnt = actualValues == null ? 0 : actualValues.size(); int passedParamCnt = actualCnt; - List routine_param_item_list = formal.create_routine_param_item(); - int formalCnt = routine_param_item_list.size(); + List routineParamItem = formal.create_routine_param_item(); + int formalCnt = routineParamItem.size(); if (actualCnt > formalCnt) { throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, actualCnt); } List defaultParamIndexes = new ArrayList<>(); if (actualCnt != formalCnt) { for (int i = 0; i < formalCnt; i++) { - if (routine_param_item_list.get(i).dtype_default() != null) { + if (routineParamItem.get(i).dtype_default() != null) { defaultParamIndexes.add(i); } } @@ -186,13 +186,13 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par scale = p.dtype_len().L_INT(1).getText(); } } - Var var = setCallParameter(name, type, len, scale, value, exec); - exec.trace(actual, "SET PARAM " + name + " = " + var.toString()); + Var variable = setCallParameter(name, type, len, scale, value, exec); + exec.trace(actual, "SET PARAM " + name + " = " + variable.toString()); if (out != null && a.expr_atom() != null && a.expr_atom().qident() != null && (p.T_OUT() != null || p.T_INOUT() != null)) { String actualName = a.expr_atom().qident().getText(); if (actualName != null) { - out.put(actualName, var); + out.put(actualName, variable); } } } From c6f0936e0875307648343438b77551e3d71b436b Mon Sep 17 00:00:00 2001 From: mdayakar Date: Wed, 21 Feb 2024 23:29:20 +0530 Subject: [PATCH 4/7] Fixed code review comments to support named parameter binding --- .../functions/InMemoryFunctionRegistry.java | 93 +++++++++++-------- .../hive/beeline/TestHplSqlViaBeeLine.java | 28 ++++++ 2 files changed, 80 insertions(+), 41 deletions(-) diff --git a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java index a30c68f658fd..68c71cec54db 100644 --- a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java +++ b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java @@ -133,10 +133,9 @@ 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 actualValues, - HplsqlParser.Create_routine_paramsContext formal, - HashMap out, - Exec exec) { + public static void setCallParameters(String procName, HplsqlParser.Expr_func_paramsContext actual, + ArrayList actualValues, HplsqlParser.Create_routine_paramsContext formal, HashMap out, + Exec exec) { // if it is a non-parameter function then just return. if (actual == null && formal == null) { return; @@ -148,52 +147,64 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par if (actualCnt > formalCnt) { throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, actualCnt); } - List defaultParamIndexes = new ArrayList<>(); + Map defaultParamNamesVsIndexes = new HashMap<>(); if (actualCnt != formalCnt) { for (int i = 0; i < formalCnt; i++) { - if (routineParamItem.get(i).dtype_default() != null) { - defaultParamIndexes.add(i); + HplsqlParser.Create_routine_param_itemContext routineParamItemContext = routineParamItem.get(i); + if (routineParamItemContext.dtype_default() != null) { + defaultParamNamesVsIndexes.put(routineParamItemContext.ident().getText(), i); } } - actualCnt = actualCnt + defaultParamIndexes.size(); + actualCnt = actualCnt + defaultParamNamesVsIndexes.size(); if (actualCnt < formalCnt) { throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, passedParamCnt); } } - for (int i = 0; i < formalCnt; i++) { - HplsqlParser.ExprContext a = null; - HplsqlParser.Create_routine_param_itemContext p = null; - Var value = null; - if (actual == null || actual.func_param() == null || actual.func_param(i) == null) { - if (!defaultParamIndexes.contains(i)) { - throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, passedParamCnt); - } - p = formal.create_routine_param_item().get(i); - a = p.dtype_default().expr(); - value = exec.evalPop(p.dtype_default().expr()); - } else { - a = actual.func_param(i).expr(); - p = getCallParameter(actual, formal, i); - value = actualValues.get(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(); - } + + HplsqlParser.ExprContext a = null; + HplsqlParser.Create_routine_param_itemContext p = null; + Var value = null; + // set the passed params + for (int i = 0; i < passedParamCnt; i++) { + a = actual.func_param(i).expr(); + p = getCallParameter(actual, formal, i); + value = actualValues.get(i); + // for any default param value is passed then remove it from default param list + defaultParamNamesVsIndexes.remove(p.ident().getText()); + setCallParameter(actual, out, exec, a, p, value); + } + // set the remaining default params + for (int index : defaultParamNamesVsIndexes.values()) { + p = formal.create_routine_param_item().get(index); + a = p.dtype_default().expr(); + value = exec.evalPop(p.dtype_default().expr()); + setCallParameter(actual, out, exec, a, p, value); + } + // if actual param count + remaining default param count is lesser than formal param count then throw exception as some params are missing + if ((passedParamCnt + defaultParamNamesVsIndexes.size()) < formalCnt) { + throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, passedParamCnt); + } + } + + private static void setCallParameter(HplsqlParser.Expr_func_paramsContext actual, HashMap out, Exec exec, + HplsqlParser.ExprContext a, HplsqlParser.Create_routine_param_itemContext p, Var value) { + 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(); } - Var variable = setCallParameter(name, type, len, scale, value, exec); - exec.trace(actual, "SET PARAM " + name + " = " + variable.toString()); - if (out != null && a.expr_atom() != null && a.expr_atom().qident() != null && - (p.T_OUT() != null || p.T_INOUT() != null)) { - String actualName = a.expr_atom().qident().getText(); - if (actualName != null) { - out.put(actualName, variable); - } + } + Var variable = setCallParameter(name, type, len, scale, value, exec); + exec.trace(actual, "SET PARAM " + name + " = " + variable.toString()); + if (out != null && a.expr_atom() != null && a.expr_atom() + .qident() != null && (p.T_OUT() != null || p.T_INOUT() != null)) { + String actualName = a.expr_atom().qident().getText(); + if (actualName != null) { + out.put(actualName, variable); } } } diff --git a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java index 617b237615c5..f35a3b0b4388 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java @@ -944,6 +944,34 @@ public void testHplSqlProcedureWithSomeDefaultValues() throws Throwable { testScriptFile(SCRIPT_TEXT, args(), "Passed_Val = 123"); } + @Test + public void testHplSqlProcedureWithDefaultParamCallingWithNamedParameterBinding() throws Throwable { + String SCRIPT_TEXT = + "DROP TABLE IF EXISTS result;\n" + + "CREATE TABLE result (s string);\n" + + "CREATE PROCEDURE p1(s STRING DEFAULT 'default_val', num NUMBER)\n" + + "BEGIN\n" + + "INSERT INTO result VALUES(s || ' = ' || num);\n" + + "END;\n" + + "p1(num => 111);\n" + + "SELECT * FROM result;" ; + testScriptFile(SCRIPT_TEXT, args(), "default_val = 111"); + } + + @Test + public void testHplSqlProcedureWithAllDefaultParamsCallingWithNamedParameterBinding() throws Throwable { + String SCRIPT_TEXT = + "DROP TABLE IF EXISTS result;\n" + + "CREATE TABLE result (s string);\n" + + "CREATE PROCEDURE p1(s1 STRING default 'Default S1', s2 string default 'Default S2')\n" + + "BEGIN\n" + + "INSERT INTO result VALUES(s1 || '=' || s2);\n" + + "END;\n" + + "p1(s2 => 'PassedValue S2');\n" + + "SELECT * FROM result;" ; + testScriptFile(SCRIPT_TEXT, args(), "Default S1=PassedValue S2"); + } + private static List args() { return Arrays.asList("-d", BeeLine.BEELINE_DEFAULT_JDBC_DRIVER, "-u", miniHS2.getBaseJdbcURL() + ";mode=hplsql", "-n", userName); From 37f5e372cd53cdca3daf178df44e96ea1b0b24a0 Mon Sep 17 00:00:00 2001 From: mdayakar Date: Thu, 22 Feb 2024 16:28:52 +0530 Subject: [PATCH 5/7] Fixed code comments --- .../functions/InMemoryFunctionRegistry.java | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java index 68c71cec54db..19527d3988e3 100644 --- a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java +++ b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java @@ -140,49 +140,55 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par if (actual == null && formal == null) { return; } - int actualCnt = actualValues == null ? 0 : actualValues.size(); + int actualCnt = (actualValues == null) ? 0 : actualValues.size(); int passedParamCnt = actualCnt; List routineParamItem = formal.create_routine_param_item(); int formalCnt = routineParamItem.size(); + ParserRuleContext ruleContext = (actual == null) ? null : actual.getParent(); if (actualCnt > formalCnt) { - throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, actualCnt); + throw new ArityException(ruleContext, procName, formalCnt, actualCnt); } Map defaultParamNamesVsIndexes = new HashMap<>(); if (actualCnt != formalCnt) { - for (int i = 0; i < formalCnt; i++) { - HplsqlParser.Create_routine_param_itemContext routineParamItemContext = routineParamItem.get(i); - if (routineParamItemContext.dtype_default() != null) { - defaultParamNamesVsIndexes.put(routineParamItemContext.ident().getText(), i); - } - } + populateDefaultParamDetails(routineParamItem, formalCnt, defaultParamNamesVsIndexes); actualCnt = actualCnt + defaultParamNamesVsIndexes.size(); if (actualCnt < formalCnt) { - throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, passedParamCnt); + throw new ArityException(ruleContext, procName, formalCnt, passedParamCnt); } } - HplsqlParser.ExprContext a = null; - HplsqlParser.Create_routine_param_itemContext p = null; + HplsqlParser.ExprContext exprContext = null; + HplsqlParser.Create_routine_param_itemContext paramItemContext = null; Var value = null; // set the passed params for (int i = 0; i < passedParamCnt; i++) { - a = actual.func_param(i).expr(); - p = getCallParameter(actual, formal, i); + exprContext = actual.func_param(i).expr(); + paramItemContext = getCallParameter(actual, formal, i); value = actualValues.get(i); // for any default param value is passed then remove it from default param list - defaultParamNamesVsIndexes.remove(p.ident().getText()); - setCallParameter(actual, out, exec, a, p, value); + defaultParamNamesVsIndexes.remove(paramItemContext.ident().getText()); + setCallParameter(actual, out, exec, exprContext, paramItemContext, value); } // set the remaining default params for (int index : defaultParamNamesVsIndexes.values()) { - p = formal.create_routine_param_item().get(index); - a = p.dtype_default().expr(); - value = exec.evalPop(p.dtype_default().expr()); - setCallParameter(actual, out, exec, a, p, value); + paramItemContext = formal.create_routine_param_item().get(index); + exprContext = paramItemContext.dtype_default().expr(); + value = exec.evalPop(paramItemContext.dtype_default().expr()); + setCallParameter(actual, out, exec, exprContext, paramItemContext, value); } // if actual param count + remaining default param count is lesser than formal param count then throw exception as some params are missing if ((passedParamCnt + defaultParamNamesVsIndexes.size()) < formalCnt) { - throw new ArityException(actual == null ? null : actual.getParent(), procName, formalCnt, passedParamCnt); + throw new ArityException(ruleContext, procName, formalCnt, passedParamCnt); + } + } + + private static void populateDefaultParamDetails(List routineParamItem, int formalCnt, + Map defaultParamNamesVsIndexes) { + for (int i = 0; i < formalCnt; i++) { + HplsqlParser.Create_routine_param_itemContext routineParamItemContext = routineParamItem.get(i); + if (routineParamItemContext.dtype_default() != null) { + defaultParamNamesVsIndexes.put(routineParamItemContext.ident().getText(), i); + } } } From 25d9c1e500c66551efad7ef9c3e4d99b53550113 Mon Sep 17 00:00:00 2001 From: mdayakar Date: Thu, 29 Feb 2024 18:13:25 +0530 Subject: [PATCH 6/7] Fixed code review comments --- .../functions/InMemoryFunctionRegistry.java | 31 +++++++------------ .../hive/beeline/TestHplSqlViaBeeLine.java | 14 +++++++++ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java index 19527d3988e3..767cb50e221a 100644 --- a/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java +++ b/hplsql/src/main/java/org/apache/hive/hplsql/functions/InMemoryFunctionRegistry.java @@ -141,7 +141,6 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par return; } int actualCnt = (actualValues == null) ? 0 : actualValues.size(); - int passedParamCnt = actualCnt; List routineParamItem = formal.create_routine_param_item(); int formalCnt = routineParamItem.size(); ParserRuleContext ruleContext = (actual == null) ? null : actual.getParent(); @@ -150,40 +149,34 @@ public static void setCallParameters(String procName, HplsqlParser.Expr_func_par } Map defaultParamNamesVsIndexes = new HashMap<>(); if (actualCnt != formalCnt) { - populateDefaultParamDetails(routineParamItem, formalCnt, defaultParamNamesVsIndexes); - actualCnt = actualCnt + defaultParamNamesVsIndexes.size(); - if (actualCnt < formalCnt) { - throw new ArityException(ruleContext, procName, formalCnt, passedParamCnt); - } + populateDefaultParamDetails(routineParamItem, defaultParamNamesVsIndexes); } - HplsqlParser.ExprContext exprContext = null; - HplsqlParser.Create_routine_param_itemContext paramItemContext = null; - Var value = null; // set the passed params - for (int i = 0; i < passedParamCnt; i++) { - exprContext = actual.func_param(i).expr(); - paramItemContext = getCallParameter(actual, formal, i); - value = actualValues.get(i); + for (int i = 0; i < actualCnt; i++) { + HplsqlParser.ExprContext exprContext = actual.func_param(i).expr(); + HplsqlParser.Create_routine_param_itemContext paramItemContext = getCallParameter(actual, formal, i); + Var value = actualValues.get(i); // for any default param value is passed then remove it from default param list defaultParamNamesVsIndexes.remove(paramItemContext.ident().getText()); setCallParameter(actual, out, exec, exprContext, paramItemContext, value); } // set the remaining default params for (int index : defaultParamNamesVsIndexes.values()) { - paramItemContext = formal.create_routine_param_item().get(index); - exprContext = paramItemContext.dtype_default().expr(); - value = exec.evalPop(paramItemContext.dtype_default().expr()); + HplsqlParser.Create_routine_param_itemContext paramItemContext = formal.create_routine_param_item().get(index); + HplsqlParser.ExprContext exprContext = paramItemContext.dtype_default().expr(); + Var value = exec.evalPop(paramItemContext.dtype_default().expr()); setCallParameter(actual, out, exec, exprContext, paramItemContext, value); } // if actual param count + remaining default param count is lesser than formal param count then throw exception as some params are missing - if ((passedParamCnt + defaultParamNamesVsIndexes.size()) < formalCnt) { - throw new ArityException(ruleContext, procName, formalCnt, passedParamCnt); + if ((actualCnt + defaultParamNamesVsIndexes.size()) != formalCnt) { + throw new ArityException(ruleContext, procName, formalCnt, actualCnt); } } - private static void populateDefaultParamDetails(List routineParamItem, int formalCnt, + private static void populateDefaultParamDetails(List routineParamItem, Map defaultParamNamesVsIndexes) { + int formalCnt = routineParamItem.size(); for (int i = 0; i < formalCnt; i++) { HplsqlParser.Create_routine_param_itemContext routineParamItemContext = routineParamItem.get(i); if (routineParamItemContext.dtype_default() != null) { diff --git a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java index f35a3b0b4388..819a5b12b021 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java @@ -972,6 +972,20 @@ public void testHplSqlProcedureWithAllDefaultParamsCallingWithNamedParameterBind testScriptFile(SCRIPT_TEXT, args(), "Default S1=PassedValue S2"); } + @Test + public void testHplSqlProcedureWithoutParameters() throws Throwable { + String SCRIPT_TEXT = + "DROP TABLE IF EXISTS result;\n" + + "CREATE TABLE result (s string);\n" + + "CREATE PROCEDURE p1()\n" + + "BEGIN\n" + + "INSERT INTO result VALUES('No param');\n" + + "END;\n" + + "p1('none');\n" + + "SELECT * FROM result;" ; + testScriptFile(SCRIPT_TEXT, args(), "wrong number of arguments in call to 'p1'. Expected 0 got 1.", OutStream.ERR); + } + private static List args() { return Arrays.asList("-d", BeeLine.BEELINE_DEFAULT_JDBC_DRIVER, "-u", miniHS2.getBaseJdbcURL() + ";mode=hplsql", "-n", userName); From 9f06021e012f02b6f92ddf6cf2acde4c5b305f50 Mon Sep 17 00:00:00 2001 From: mdayakar Date: Thu, 29 Feb 2024 21:24:19 +0530 Subject: [PATCH 7/7] Fixed test failures --- hplsql/src/test/results/local/arity.out.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/hplsql/src/test/results/local/arity.out.txt b/hplsql/src/test/results/local/arity.out.txt index ccf4e589e752..43542c011a27 100644 --- a/hplsql/src/test/results/local/arity.out.txt +++ b/hplsql/src/test/results/local/arity.out.txt @@ -7,4 +7,5 @@ a=1 Ln:4 PRINT b=2 Ln:8 EXEC PROCEDURE P +Ln:8 SET PARAM a = 1 Ln:8 wrong number of arguments in call to 'P'. Expected 2 got 1.