Commit 271e03cc authored by gmantele's avatar gmantele
Browse files

[ADQL,TAP] Fix bug (reported by G. Landais) in the understanding of UNKNOWN 

types. The notion of "unknown type" is different in function of the target
object:
  - a DBType and a FunctionDef have an unknown type if their function
    isUnknown() returns true. In such case, the other functions such as
	isNumeric/String/Geometry() will return false.
  - an ADQLOperand (e.g. ADQLColumn) does NOT have a isUnknown() function.
    But if the type of the operand is unknown, its functions isNumeric(),
	isString() and isGeometry() must ALL return true. Otherwise, just one of
	these functions can return true.
parent 13a2dc54

src/adql/db/DBType.java

+10 −9
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ package adql.db; 
 * It is used to set the attribute type/datatype of this class.</p>

 *  

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

 * @version 1.4 (07/2015)

 * @version 1.4 (08/2015)

 * @since 1.3

 */

public class DBType {
@@ -119,7 +119,8 @@ public class DBType { 
	 * 

	 * <p><i><b>Important note</b>:

	 * 	Since {@link DBDatatype#UNKNOWN UNKNOWN} is an unresolved type, it can potentially be anything.

	 * 	That's why, this function will also returned <code>true</code> if the type is

	 * 	But, in order to avoid incorrect operation while expecting a numeric although the type is unknown

	 * 	and is in fact not really a numeric, this function will return <code>false</code> if the type is

	 * 	{@link DBDatatype#UNKNOWN UNKNOWN}.

	 * </i></p>

	 *
@@ -137,7 +138,6 @@ public class DBType { 
			case BINARY:

			case VARBINARY:

			case BLOB:

			case UNKNOWN:

				return true;

			default:

				return false;
@@ -153,7 +153,8 @@ public class DBType { 
	 * 

	 * <p><i><b>Important note</b>:

	 * 	Since {@link DBDatatype#UNKNOWN UNKNOWN} is an unresolved type, it can potentially be anything.

	 * 	That's why, this function will also returned <code>true</code> if the type is

	 * 	But, in order to avoid incorrect operation while expecting a binary although the type is unknown

	 * 	and is in fact not really a binary, this function will return <code>false</code> if the type is

	 * 	{@link DBDatatype#UNKNOWN UNKNOWN}.

	 * </i></p>

	 *
@@ -164,7 +165,6 @@ public class DBType { 
			case BINARY:

			case VARBINARY:

			case BLOB:

			case UNKNOWN:

				return true;

			default:

				return false;
@@ -181,7 +181,8 @@ public class DBType { 
	 * 

	 * <p><i><b>Important note</b>:

	 * 	Since {@link DBDatatype#UNKNOWN UNKNOWN} is an unresolved type, it can potentially be anything.

	 * 	That's why, this function will also returned <code>true</code> if the type is

	 * 	But, in order to avoid incorrect operation while expecting a string although the type is unknown

	 * 	and is in fact not really a string, this function will return <code>false</code> if the type is

	 * 	{@link DBDatatype#UNKNOWN UNKNOWN}.

	 * </i></p>

	 *
@@ -193,7 +194,6 @@ public class DBType { 
			case VARCHAR:

			case CLOB:

			case TIMESTAMP:

			case UNKNOWN:

				return true;

			default:

				return false;
@@ -209,14 +209,15 @@ public class DBType { 
	 * 

	 * <p><i><b>Important note</b>:

	 * 	Since {@link DBDatatype#UNKNOWN UNKNOWN} is an unresolved type, it can potentially be anything.

	 * 	That's why, this function will also returned <code>true</code> if the type is

	 * 	But, in order to avoid incorrect operation while expecting a geometry although the type is unknown

	 * 	and is in fact not really a geometry, this function will return <code>false</code> if the type is

	 * 	{@link DBDatatype#UNKNOWN UNKNOWN}.

	 * </i></p>

	 * 

	 * @return	<code>true</code> if this type is a geometry, <code>false</code> otherwise.

	 */

	public boolean isGeometry(){

		return (type == DBDatatype.POINT || type == DBDatatype.REGION || type == DBDatatype.UNKNOWN);

		return (type == DBDatatype.POINT || type == DBDatatype.REGION);

	}



	/**

src/adql/db/FunctionDef.java

+17 −6
Original line number Diff line number Diff line
@@ -201,9 +201,9 @@ public class FunctionDef implements Comparable<FunctionDef> { 
		// Set the return type;

		this.returnType = (returnType != null) ? returnType : new DBType(DBDatatype.UNKNOWN);

		isUnknown = this.returnType.isUnknown();

		isNumeric = isUnknown || this.returnType.isNumeric();

		isString = isUnknown || this.returnType.isString();

		isGeometry = isUnknown || this.returnType.isGeometry();

		isNumeric = this.returnType.isNumeric();

		isString = this.returnType.isString();

		isGeometry = this.returnType.isGeometry();



		// Serialize in Strings (serializedForm and compareForm) this function definition:

		StringBuffer bufSer = new StringBuffer(name), bufCmp = new StringBuffer(name.toLowerCase());
@@ -515,7 +515,7 @@ public class FunctionDef implements Comparable<FunctionDef> { 
	 * 	not part of a function signature.

	 * </p>

	 * 

	 * <p>The notion of "greater" and "less" are defined here according to the three following test steps:</p>

	 * <p>The notions of "greater" and "less" are defined here according to the three following test steps:</p>

	 * <ol>

	 * 	<li><b>Name test:</b> if the name of both function are equals, next steps are evaluated, otherwise the standard string comparison (case insensitive) result is returned.</li>

	 * 	<li><b>Parameters test:</b> parameters are compared individually. Each time parameters (at the same position in both functions) are equals the next parameter can be tested,
@@ -529,10 +529,19 @@ public class FunctionDef implements Comparable<FunctionDef> { 
	 * 	                                      returned. Otherwise a negative value will be returned, or 0 if the number of parameters is the same.</li>

	 * </ol>

	 * 

	 * <p><i><b>Note:</b>

	 * 	If one of the tested types (i.e. parameters types) is unknown, the match should return 0 (i.e. equality).

	 * 	The notion of "unknown" is different in function of the tested item. A {@link DBType} is unknown if its function

	 * 	{@link DBType#isUnknown()} returns <code>true</code> ; thus, its other functions such as {@link DBType#isNumeric()} will

	 * 	return <code>false</code>. On the contrary, an {@link ADQLOperand} does not have any isUnknown()

	 * 	function. However, when the type of a such is unknown, all its functions isNumeric(), isString() and isGeometry() return

	 * 	<code>true</code>.

	 * </i></p>

	 * 

	 * @param fct	ADQL function item to compare with this function definition.

	 * 

	 * @return	A positive value if this function definition is "greater" than the given {@link ADQLFunction},

	 * 			0 if they are perfectly matching,

	 * 			0 if they are perfectly matching or one of the tested types (i.e. parameters types) is unknown,

	 * 			or a negative value if this function definition is "less" than the given {@link ADQLFunction}.

	 */

	public int compareTo(final ADQLFunction fct){
@@ -545,8 +554,10 @@ public class FunctionDef implements Comparable<FunctionDef> { 
		// If equals, compare the parameters' type:

		if (comp == 0){

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

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

				// if one of the types is unknown, the comparison should return true:

				if (params[i].type.isUnknown() || (fct.getParameter(i).isNumeric() && fct.getParameter(i).isString() && fct.getParameter(i).isGeometry()))

					comp = 0;

				// otherwise, just compare each kind of type for an exact match:

				else if (params[i].type.isNumeric() == fct.getParameter(i).isNumeric()){

					if (params[i].type.isString() == fct.getParameter(i).isString()){

						if (params[i].type.isGeometry() == fct.getParameter(i).isGeometry())

src/adql/query/operand/ADQLColumn.java

+3 −3
Original line number Diff line number Diff line
@@ -469,17 +469,17 @@ public class ADQLColumn implements ADQLOperand, UnknownType { 


	@Override

	public boolean isNumeric(){

		return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isNumeric());

		return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isUnknown() || dbLink.getDatatype().isNumeric());

	}



	@Override

	public boolean isString(){

		return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isString());

		return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isUnknown() || dbLink.getDatatype().isString());

	}



	@Override

	public boolean isGeometry(){

		return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isGeometry());

		return (dbLink == null || dbLink.getDatatype() == null || dbLink.getDatatype().isUnknown() || dbLink.getDatatype().isGeometry());

	}



	@Override

src/tap/formatter/JSONFormat.java

+2 −2
Original line number Diff line number Diff line
@@ -42,7 +42,7 @@ import adql.db.DBType.DBDatatype; 
 * Format any given query (table) result into JSON.

 * 

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

 * @version 2.1 (07/2015)

 * @version 2.1 (08/2015)

 */

public class JSONFormat implements OutputFormat {
@@ -180,7 +180,7 @@ public class JSONFormat implements OutputFormat { 
	protected TAPColumn getValidColMeta(final DBColumn typeFromQuery, final TAPColumn typeFromResult){

		if (typeFromQuery != null && typeFromQuery instanceof TAPColumn){

			TAPColumn colMeta = (TAPColumn)typeFromQuery;

			if (colMeta.getDatatype().type == DBDatatype.UNKNOWN && typeFromResult != null && typeFromResult.getDatatype().type != DBDatatype.UNKNOWN)

			if (colMeta.getDatatype().isUnknown() && typeFromResult != null && !typeFromResult.getDatatype().isUnknown())

				colMeta.setDatatype(typeFromResult.getDatatype());

			return colMeta;

		}else if (typeFromResult != null){

src/tap/formatter/VOTableFormat.java

+1 −1
Original line number Diff line number Diff line
@@ -464,7 +464,7 @@ public class VOTableFormat implements OutputFormat { 
	protected static final TAPColumn getValidColMeta(final DBColumn typeFromQuery, final TAPColumn typeFromResult){

		if (typeFromQuery != null && typeFromQuery instanceof TAPColumn){

			TAPColumn colMeta = (TAPColumn)typeFromQuery;

			if (colMeta.getDatatype().type == DBDatatype.UNKNOWN && typeFromResult != null && typeFromResult.getDatatype().type != DBDatatype.UNKNOWN)

			if (colMeta.getDatatype().isUnknown() && typeFromResult != null && !typeFromResult.getDatatype().isUnknown())

				colMeta.setDatatype(typeFromResult.getDatatype());

			return colMeta;

		}else if (typeFromResult != null){