From 475fcb6508ff2482fb2c5a186c2731a7acefa34b Mon Sep 17 00:00:00 2001
From: gmantele <gmantele@ari.uni-heidelberg.de>
Date: Fri, 4 Mar 2016 20:03:58 +0100
Subject: [PATCH] [ADQL] Fix identification of UDFs using a list of function
definitions. Functions whose some parameters are another function were not
correctly identified: since the inner functions were not yet identified,
their type was UNKNOWN and so it makes the identification of the parent
function much easier since an UNKNOWN parameter is not checked. But, it was a
problem if the parameter occurs to be finally of the wrong type.
---
src/adql/db/DBChecker.java | 36 +++++++++++++++++----------------
test/adql/db/TestDBChecker.java | 25 +++++++++++++++++++++++
2 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/src/adql/db/DBChecker.java b/src/adql/db/DBChecker.java
index cc5fe8d..fa78566 100644
--- a/src/adql/db/DBChecker.java
+++ b/src/adql/db/DBChecker.java
@@ -16,8 +16,8 @@ package adql.db;
* You should have received a copy of the GNU Lesser General Public License
* along with ADQLLibrary. If not, see <http://www.gnu.org/licenses/>.
*
- * Copyright 2011,2013-2015 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
- * Astronomisches Rechen Institut (ARI)
+ * Copyright 2011-2016 - UDS/Centre de Données astronomiques de Strasbourg (CDS),
+ * Astronomisches Rechen Institut (ARI)
*/
import java.lang.reflect.Constructor;
@@ -97,7 +97,7 @@ import adql.search.SimpleSearchHandler;
* </i></p>
*
* @author Grégory Mantelet (CDS;ARI)
- * @version 1.4 (08/2015)
+ * @version 1.4 (03/2016)
*/
public class DBChecker implements QueryChecker {
@@ -813,24 +813,26 @@ public class DBChecker implements QueryChecker {
* for a later resolution try. */
for(ADQLObject result : sHandler){
udf = (UserDefinedFunction)result;
- // search for a match:
- match = binSearch.search(udf, allowedUdfs);
- // if no match...
- if (match < 0){
- // ...if the type of all parameters is resolved, add an error (no match is possible):
- if (isAllParamTypesResolved(udf))
+ // if the type of not all parameters are resolved, postpone the resolution:
+ if (!isAllParamTypesResolved(udf))
+ toResolveLater.add(udf);
+ // otherwise:
+ else{
+ // search for a match:
+ match = binSearch.search(udf, allowedUdfs);
+ // if no match...
+ if (match < 0)
errors.addException(new UnresolvedFunctionException(udf));
- // ...otherwise, try to resolved it later (when other UDFs will be mostly resolved):
- else
- toResolveLater.add(udf);
+ // if there is a match, metadata may be attached (particularly if the function is built automatically by the syntactic parser):
+ else if (udf instanceof DefaultUDF)
+ ((DefaultUDF)udf).setDefinition(allowedUdfs[match]);
}
- // if there is a match, metadata may be attached (particularly if the function is built automatically by the syntactic parser):
- else if (udf instanceof DefaultUDF)
- ((DefaultUDF)udf).setDefinition(allowedUdfs[match]);
}
// Try to resolve UDFs whose some parameter types are depending of other UDFs:
- for(int i = 0; i < toResolveLater.size(); i++){
+ /* Note: we need to iterate from the end in order to resolve first the most wrapped functions
+ * (e.g. fct1(fct2(...)) ; fct2 must be resolved before fct1). */
+ for(int i = toResolveLater.size() - 1; i >= 0; i--){
udf = toResolveLater.get(i);
// search for a match:
match = binSearch.search(udf, allowedUdfs);
@@ -866,7 +868,7 @@ public class DBChecker implements QueryChecker {
*/
protected final boolean isAllParamTypesResolved(final ADQLFunction fct){
for(ADQLOperand op : fct.getParameters()){
- if (op.isNumeric() == op.isString())
+ if (op.isGeometry() == op.isNumeric() && op.isNumeric() == op.isString())
return false;
}
return true;
diff --git a/test/adql/db/TestDBChecker.java b/test/adql/db/TestDBChecker.java
index b761381..36799c8 100644
--- a/test/adql/db/TestDBChecker.java
+++ b/test/adql/db/TestDBChecker.java
@@ -675,6 +675,31 @@ public class TestDBChecker {
assertEquals("Type mismatch! A numeric value was expected instead of \"titi()\".", ex.getErrors().next().getMessage());
}
+ // Try with functions wrapped on 2 levels:
+ // i.e. fct1('blabla', fct2(fct3('blabla')))
+ FunctionDef[] complexFcts = new FunctionDef[3];
+ complexFcts[0] = new FunctionDef("fct1", new DBType(DBDatatype.VARCHAR), new FunctionParam[]{new FunctionParam("str", new DBType(DBDatatype.VARCHAR)),new FunctionParam("num", new DBType(DBDatatype.INTEGER))});
+ complexFcts[1] = new FunctionDef("fct2", new DBType(DBDatatype.INTEGER), new FunctionParam[]{new FunctionParam("str", new DBType(DBDatatype.VARCHAR))});
+ complexFcts[2] = new FunctionDef("fct3", new DBType(DBDatatype.VARCHAR), new FunctionParam[]{new FunctionParam("str", new DBType(DBDatatype.VARCHAR))});
+ parser = new ADQLParser(new DBChecker(tables, Arrays.asList(complexFcts)));
+ // With parameters of the good type:
+ try{
+ assertNotNull(parser.parseQuery("SELECT fct1('blabla', fct2(fct3('blabla'))) FROM foo"));
+ }catch(ParseException pe){
+ pe.printStackTrace();
+ fail("Types are matching: this test should have succeeded!");
+ }
+ // With parameters of the bad type:
+ try{
+ parser.parseQuery("SELECT fct2(fct1('blabla', fct3('blabla'))) FROM foo");
+ fail("Parameters types are not matching: the parsing should have failed!");
+ }catch(ParseException pe){
+ assertEquals(UnresolvedIdentifiersException.class, pe.getClass());
+ assertEquals(1, ((UnresolvedIdentifiersException)pe).getNbErrors());
+ ParseException innerPe = ((UnresolvedIdentifiersException)pe).getErrors().next();
+ assertEquals("Unresolved function: \"fct1('blabla', fct3('blabla'))\"! No UDF has been defined or found with the signature: fct1(STRING, STRING).", innerPe.getMessage());
+ }
+
// CLEAR ALL UDFs AND ALLOW UNKNOWN FUNCTION:
parser = new ADQLParser(new DBChecker(tables, null));
--
GitLab