Commit b49f8160 authored by gmantele's avatar gmantele
Browse files

[ADQL,TAP] Fix 2 bugs notified by M.Taylor: 

1/ Bad parsing of UDF's parameter type or return type. Database types whose the
name has a space (e.g. 'double precision') were not accepted. These type names
should be accepted according to TAPRegExt.
2/ An error message thrown by the DBChecker has been modified for the cases
the type of a parameter is unknown. Before the returned type was NUMERIC. Now,
it will be 'param' followed by the parameter index (e.g. 'param1').
parent 1234f1a1

src/adql/db/FunctionDef.java

+19 −15
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ import adql.query.operand.function.UserDefinedFunction; 
 * </p>

 * 

 * @author Gr&eacute;gory Mantelet (ARI)

 * @version 1.4 (07/2015)

 * @version 1.4 (08/2015)

 * 

 * @since 1.3

 */
@@ -348,10 +348,12 @@ public class FunctionDef implements Comparable<FunctionDef> { 
	 * <pre>{fctName}([{param1Name} {param1Type}, ...])[ -> {returnType}]</pre>

	 * 

	 * <p>

	 * 	Allowed parameter types and return types should be one the types listed by the UPLOAD section of the TAP recommendation document.

	 * 	<em>This function must be able to parse functions as defined by TAPRegExt (section 2.3).</em>

	 * 	Hence, allowed parameter types and return types should be one of the types listed by the UPLOAD section of the TAP recommendation document.

	 * 	These types are listed in the enumeration object {@link DBType}.

	 * 	However, other types should be accepted like the common database types...but it should be better to not rely on that

	 * 	since the conversion of those types to TAP types should not be exactly what is expected.

	 * 	since the conversion of those types to TAP types should not be exactly what is expected (because depending from the used DBMS);

	 *  a default interpretation of database types is nevertheless processed by this parser.

	 * </p>

	 * 

	 * @param strDefinition	Serialized function definition to parse.
@@ -426,7 +428,8 @@ public class FunctionDef implements Comparable<FunctionDef> { 
	 * 

	 * @param datatype	String representation of a datatype.

	 *                	<i>Note: This string must not contain the length parameter or any other parameter.

	 *                	These latter should have been separated from the datatype before calling this function.</i>

	 *                	These latter should have been separated from the datatype before calling this function.

	 *                	It can however contain space(s) in first, last or intern position.</i>

	 * @param length	Length of this datatype.

	 *              	<i>Note: This length will be used only for binary (BINARY and VARBINARY)

	 *              	and character (CHAR and VARCHAR) types.</i>
@@ -438,6 +441,9 @@ public class FunctionDef implements Comparable<FunctionDef> { 
		if (datatype == null)

			return null;



		// Remove leading and trailing spaces and replace each inner serie of spaces by just one space:

		datatype = datatype.trim().replaceAll(" +", " ");



		try{

			// Try to find a corresponding DBType item:

			DBDatatype dbDatatype = DBDatatype.valueOf(datatype.toUpperCase());
@@ -456,31 +462,29 @@ public class FunctionDef implements Comparable<FunctionDef> { 
		}catch(IllegalArgumentException iae){

			// If there's no corresponding DBType item, try to find a match among the most used DB types:

			datatype = datatype.toLowerCase();

			if (datatype.equals("bool") || datatype.equals("boolean") || datatype.equals("short"))

				return new DBType(DBDatatype.SMALLINT);

			else if (datatype.equals("int2"))

			if (datatype.equals("bool") || datatype.equals("boolean") || datatype.equals("short") || datatype.equals("int2") || datatype.equals("smallserial") || datatype.equals("serial2"))

				return new DBType(DBDatatype.SMALLINT);

			else if (datatype.equals("int") || datatype.equals("int4"))

			else if (datatype.equals("int") || datatype.equals("int4") || datatype.equals("serial") || datatype.equals("serial4"))

				return new DBType(DBDatatype.INTEGER);

			else if (datatype.equals("long") || datatype.equals("number") || datatype.equals("bigint") || datatype.equals("int8"))

			else if (datatype.equals("long") || datatype.equals("number") || datatype.equals("int8") || datatype.equals("bigserial") || datatype.equals("bigserial8"))

				return new DBType(DBDatatype.BIGINT);

			else if (datatype.equals("float") || datatype.equals("float4"))

				return new DBType(DBDatatype.REAL);

			else if (datatype.equals("numeric") || datatype.equals("float8"))

			else if (datatype.equals("numeric") || datatype.equals("float8") || datatype.equals("double precision"))

				return new DBType(DBDatatype.DOUBLE);

			else if (datatype.equals("byte") || datatype.equals("raw"))

			else if (datatype.equals("bit") || datatype.equals("byte") || datatype.equals("raw"))

				return new DBType(DBDatatype.BINARY, length);

			else if (datatype.equals("unsignedByte"))

			else if (datatype.equals("unsignedByte") || datatype.equals("bit varying") || datatype.equals("varbit"))

				return new DBType(DBDatatype.VARBINARY, length);

			else if (datatype.equals("character"))

				return new DBType(DBDatatype.CHAR, length);

			else if (datatype.equals("string") || datatype.equals("varchar2"))

			else if (datatype.equals("string") || datatype.equals("varchar2") || datatype.equals("character varying"))

				return new DBType(DBDatatype.VARCHAR, length);

			else if (datatype.equals("bytea"))

				return new DBType(DBDatatype.BLOB);

			else if (datatype.equals("text"))

				return new DBType(DBDatatype.CLOB);

			else if (datatype.equals("date") || datatype.equals("time"))

			else if (datatype.equals("date") || datatype.equals("time") || datatype.equals("timetz") || datatype.equals("timestamptz"))

				return new DBType(DBDatatype.TIMESTAMP);

			else if (datatype.equals("position"))

				return new DBType(DBDatatype.POINT);

src/adql/db/exception/UnresolvedFunctionException.java

+7 −4
Original line number Diff line number Diff line
@@ -27,7 +27,7 @@ import adql.query.operand.function.ADQLFunction; 
 * Exception thrown when a function can not be resolved by the library.

 * 

 * @author Gr&eacute;gory Mantelet (ARI)

 * @version 1.4 (06/2015)

 * @version 1.4 (08/2015)

 * @since 1.3

 */

public class UnresolvedFunctionException extends ParseException {
@@ -100,7 +100,8 @@ public class UnresolvedFunctionException extends ParseException { 
	 * 

	 * <p><i>Note 1:

	 * 	A parameter type can be either "NUMERIC", "STRING" or "GEOMETRY". In order to be the most generic has possible,

	 * 	no more precision about a type is returned here. If the parameter is none of these type kinds, "???" is returned.

	 * 	no more precision about a type is returned here. If the parameter is none of these type kinds, "param" suffixed

	 * 	by the parameter index (e.g. "param1") is returned.

	 * </i></p>

	 * 

	 * <p><i>Note 2:
@@ -118,14 +119,16 @@ public class UnresolvedFunctionException extends ParseException { 
		StringBuffer buf = new StringBuffer(fct.getName().toLowerCase());

		buf.append('(');

		for(int i = 0; i < fct.getNbParameters(); i++){

			if (fct.getParameter(i).isNumeric())

			if (fct.getParameter(i).isNumeric() && fct.getParameter(i).isString() && fct.getParameter(i).isGeometry())

				buf.append("param").append(i + 1);

			else if (fct.getParameter(i).isNumeric())

				buf.append("NUMERIC");

			else if (fct.getParameter(i).isString())

				buf.append("STRING");

			else if (fct.getParameter(i).isGeometry())

				buf.append("GEOMETRY");

			else

				buf.append("???");

				buf.append("param").append(i + 1);



			if ((i + 1) < fct.getNbParameters())

				buf.append(", ");

test/adql/db/TestDBChecker.java

+24 −0
Original line number Diff line number Diff line
@@ -155,6 +155,30 @@ public class TestDBChecker { 
			assertEquals("Unresolved function: \"toto('blabla')\"! No UDF has been defined or found with the signature: toto(STRING).", ex.getErrors().next().getMessage());

		}



		// Test but with at least one column parameter:

		try{

			parser.parseQuery("SELECT toto(colS) FROM foo;");

			fail("This query contains an unknown UDF signature (the fct toto is declared with no parameter): this test should have failed!");

		}catch(ParseException e){

			assertTrue(e instanceof UnresolvedIdentifiersException);

			UnresolvedIdentifiersException ex = (UnresolvedIdentifiersException)e;

			assertEquals(1, ex.getNbErrors());

			assertEquals("Unresolved function: \"toto(colS)\"! No UDF has been defined or found with the signature: toto(STRING).", ex.getErrors().next().getMessage());

		}



		// Test but with at least one unknown column parameter:

		try{

			parser.parseQuery("SELECT toto(whatami) FROM foo;");

			fail("This query contains an unknown UDF signature (the fct toto is declared with no parameter): this test should have failed!");

		}catch(ParseException e){

			assertTrue(e instanceof UnresolvedIdentifiersException);

			UnresolvedIdentifiersException ex = (UnresolvedIdentifiersException)e;

			assertEquals(2, ex.getNbErrors());

			Iterator<ParseException> errors = ex.getErrors();

			assertEquals("Unknown column \"whatami\" !", errors.next().getMessage());

			assertEquals("Unresolved function: \"toto(whatami)\"! No UDF has been defined or found with the signature: toto(param1).", errors.next().getMessage());

		}



		// Test with a UDF whose the class is specified ; the corresponding object in the ADQL tree must be replace by an instance of this class:

		udfs = new FunctionDef[]{new FunctionDef("toto", new DBType(DBDatatype.VARCHAR), new FunctionParam[]{new FunctionParam("txt", new DBType(DBDatatype.VARCHAR))})};

		udfs[0].setUDFClass(UDFToto.class);

test/adql/db/TestFunctionDef.java

+14 −0
Original line number Diff line number Diff line
@@ -135,6 +135,20 @@ public class TestFunctionDef { 
			fail("Wrong type parsing!");

		}



		// TYPE WITH SPACES AND/OR PARAMETER test:

		try{

			assertEquals("foo() -> DOUBLE", FunctionDef.parse("foo() -> double precision").toString());

			assertEquals("foo(bar DOUBLE)", FunctionDef.parse("foo(bar   DOUBLE  Precision  )").toString());

			assertEquals("foo() -> VARCHAR", FunctionDef.parse("foo() -> character varying").toString());

			assertEquals("foo(bar VARBINARY)", FunctionDef.parse("foo(bar bit   varying)").toString());

			assertEquals("foo(bar VARCHAR(12))", FunctionDef.parse("foo(bar varchar  (12))").toString());

			assertEquals("foo(bar VARCHAR(12))", FunctionDef.parse("foo(bar character varying (12))").toString());

			assertEquals("foo() -> DOUBLE", FunctionDef.parse("foo() -> double precision (2)").toString());

		}catch(Exception ex){

			ex.printStackTrace(System.err);

			fail("Wrong type parsing!");

		}



		// WRONG string definitions:

		try{

			FunctionDef.parse("123()");