From 163ec172d10648be90196b8cd619025eaa21483f Mon Sep 17 00:00:00 2001
From: gmantele <gmantele@ari.uni-heidelberg.de>
Date: Tue, 9 Jun 2015 14:04:44 +0200
Subject: [PATCH] [ADQL] Specify position while building
UnresolvedFunctionException and UnresolvedJoinException.
---
src/adql/db/DBChecker.java | 39 ++++---
src/adql/db/STCS.java | 4 +-
.../UnresolvedFunctionException.java | 21 +++-
.../db/exception/UnresolvedJoinException.java | 15 ++-
src/adql/query/from/ADQLJoin.java | 105 +++++++++---------
test/adql/db/TestDBChecker.java | 10 +-
6 files changed, 118 insertions(+), 76 deletions(-)
diff --git a/src/adql/db/DBChecker.java b/src/adql/db/DBChecker.java
index 5aeefc4..05c3b00 100644
--- a/src/adql/db/DBChecker.java
+++ b/src/adql/db/DBChecker.java
@@ -97,7 +97,7 @@ import adql.search.SimpleSearchHandler;
* </i></p>
*
* @author Grégory Mantelet (CDS;ARI)
- * @version 1.3 (05/2015)
+ * @version 1.4 (06/2015)
*/
public class DBChecker implements QueryChecker {
@@ -215,8 +215,8 @@ public class DBChecker implements QueryChecker {
tmp[cnt++] = udf;
}
// make a copy of the array:
- this.allowedUdfs = new FunctionDef[cnt];
- System.arraycopy(tmp, 0, this.allowedUdfs, 0, cnt);
+ this.allowedUdfs = new FunctionDef[cnt];
+ System.arraycopy(tmp, 0, this.allowedUdfs, 0, cnt);
tmp = null;
// sort the values:
@@ -321,7 +321,7 @@ public class DBChecker implements QueryChecker {
// Make an adjusted array copy:
String[] copy = new String[cnt];
- System.arraycopy(tmp, 0, copy, 0, cnt);
+ System.arraycopy(tmp, 0, copy, 0, cnt);
// Sort the values:
Arrays.sort(copy);
@@ -819,7 +819,7 @@ public class DBChecker implements QueryChecker {
if (match < 0){
// ...if the type of all parameters is resolved, add an error (no match is possible):
if (isAllParamTypesResolved(udf))
- errors.addException(new UnresolvedFunctionException(udf)); // TODO Add the ADQLOperand position!
+ errors.addException(new UnresolvedFunctionException(udf));
// ...otherwise, try to resolved it later (when other UDFs will be mostly resolved):
else
toResolveLater.add(udf);
@@ -836,7 +836,7 @@ public class DBChecker implements QueryChecker {
match = binSearch.search(udf, allowedUdfs);
// if no match, add an error:
if (match < 0)
- errors.addException(new UnresolvedFunctionException(udf)); // TODO Add the ADQLOperand position!
+ errors.addException(new UnresolvedFunctionException(udf));
// otherwise, 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]);
@@ -1003,7 +1003,7 @@ public class DBChecker implements QueryChecker {
try{
checkCoordinateSystem(STCS.parseCoordSys(coordSysStr), adqlCoordSys, errors);
}catch(ParseException pe){
- errors.addException(new ParseException(pe.getMessage())); // TODO Missing object position!
+ errors.addException(new ParseException(pe.getMessage(), adqlCoordSys.getPosition()));
}
}
@@ -1017,8 +1017,21 @@ public class DBChecker implements QueryChecker {
* @since 1.3
*/
protected void checkCoordinateSystem(final CoordSys coordSys, final ADQLOperand operand, final UnresolvedIdentifiersException errors){
- if (coordSysRegExp != null && coordSys != null && !coordSys.toFullSTCS().matches(coordSysRegExp))
- errors.addException(new ParseException("Coordinate system \"" + ((operand instanceof StringConstant) ? ((StringConstant)operand).getValue() : coordSys.toString()) + "\" (= \"" + coordSys.toFullSTCS() + "\") not allowed in this implementation.")); // TODO Missing object position! + List of accepted coordinate systems
+ if (coordSysRegExp != null && coordSys != null && !coordSys.toFullSTCS().matches(coordSysRegExp)){
+ StringBuffer buf = new StringBuffer();
+ if (allowedCoordSys != null){
+ for(String cs : allowedCoordSys){
+ if (buf.length() > 0)
+ buf.append(", ");
+ buf.append(cs);
+ }
+ }
+ if (buf.length() == 0)
+ buf.append("No coordinate system is allowed!");
+ else
+ buf.insert(0, "Allowed coordinate systems are: ");
+ errors.addException(new ParseException("Coordinate system \"" + ((operand instanceof StringConstant) ? ((StringConstant)operand).getValue() : coordSys.toString()) + "\" (= \"" + coordSys.toFullSTCS() + "\") not allowed in this implementation. " + buf.toString(), operand.getPosition()));
+ }
}
/**
@@ -1060,7 +1073,7 @@ public class DBChecker implements QueryChecker {
// check whether the regions (this one + the possible inner ones) and the coordinate systems are allowed:
checkRegion(region, (RegionFunction)result, binSearch, errors);
}catch(ParseException pe){
- errors.addException(new ParseException(pe.getMessage())); // TODO Missing object position!
+ errors.addException(new ParseException(pe.getMessage(), result.getPosition()));
}
}
}
@@ -1155,17 +1168,17 @@ public class DBChecker implements QueryChecker {
case 'G':
case 'g':
if (!unknown.isGeometry())
- errors.addException(new ParseException("Type mismatch! A geometry was expected instead of \"" + unknown.toADQL() + "\".")); // TODO Add the ADQLOperand position!
+ errors.addException(new ParseException("Type mismatch! A geometry was expected instead of \"" + unknown.toADQL() + "\".", result.getPosition()));
break;
case 'N':
case 'n':
if (!unknown.isNumeric())
- errors.addException(new ParseException("Type mismatch! A numeric value was expected instead of \"" + unknown.toADQL() + "\".")); // TODO Add the ADQLOperand position!
+ errors.addException(new ParseException("Type mismatch! A numeric value was expected instead of \"" + unknown.toADQL() + "\".", result.getPosition()));
break;
case 'S':
case 's':
if (!unknown.isString())
- errors.addException(new ParseException("Type mismatch! A string value was expected instead of \"" + unknown.toADQL() + "\".")); // TODO Add the ADQLOperand position!
+ errors.addException(new ParseException("Type mismatch! A string value was expected instead of \"" + unknown.toADQL() + "\".", result.getPosition()));
break;
}
}
diff --git a/src/adql/db/STCS.java b/src/adql/db/STCS.java
index 716ef58..b0befa1 100644
--- a/src/adql/db/STCS.java
+++ b/src/adql/db/STCS.java
@@ -1395,7 +1395,7 @@ public final class STCS {
if (nextToken().matches(numericRegExp))
return Double.parseDouble(token);
else
- throw new ParseException("a numeric was expected!", new TextPosition(1, pos - token.length(), 1, pos)); // TODO Check the begin and end!
+ throw new ParseException("a numeric was expected!", new TextPosition(1, pos - token.length(), 1, pos));
}
/**
@@ -1415,7 +1415,7 @@ public final class STCS {
if (pe instanceof EOEException)
throw pe;
else
- throw new ParseException("a coordinates pair (2 numerics separated by one or more spaces) was expected!", new TextPosition(1, startPos, 1, pos)); // TODO Check the begin and end!
+ throw new ParseException("a coordinates pair (2 numerics separated by one or more spaces) was expected!", new TextPosition(1, startPos, 1, pos));
}
}
diff --git a/src/adql/db/exception/UnresolvedFunctionException.java b/src/adql/db/exception/UnresolvedFunctionException.java
index befb556..4551159 100644
--- a/src/adql/db/exception/UnresolvedFunctionException.java
+++ b/src/adql/db/exception/UnresolvedFunctionException.java
@@ -20,13 +20,14 @@ package adql.db.exception;
*/
import adql.parser.ParseException;
+import adql.query.TextPosition;
import adql.query.operand.function.ADQLFunction;
/**
* Exception thrown when a function can not be resolved by the library.
*
* @author Grégory Mantelet (ARI)
- * @version 1.3 (05/2015)
+ * @version 1.4 (06/2015)
* @since 1.3
*/
public class UnresolvedFunctionException extends ParseException {
@@ -41,7 +42,19 @@ public class UnresolvedFunctionException extends ParseException {
* @param message Description of the error.
*/
public UnresolvedFunctionException(final String message){
- super(message);
+ this(message, (TextPosition)null);
+ }
+
+ /**
+ * Build the exception with just a message.
+ *
+ * @param message Description of the error.
+ * @param pos Position of the unresolved function inside the ADQL query.
+ *
+ * @since 1.4
+ */
+ public UnresolvedFunctionException(final String message, final TextPosition pos){
+ super(message, pos);
functionInError = null;
}
@@ -52,7 +65,7 @@ public class UnresolvedFunctionException extends ParseException {
* @param fct The unresolved function.
*/
public UnresolvedFunctionException(final ADQLFunction fct){
- super("Unresolved function: \"" + fct.toADQL() + "\"! No UDF has been defined or found with the signature: " + getFctSignature(fct) + "."); // TODO Add the position of the function in the ADQL query!
+ super("Unresolved function: \"" + fct.toADQL() + "\"! No UDF has been defined or found with the signature: " + getFctSignature(fct) + ".", fct.getPosition());
functionInError = fct;
}
@@ -64,7 +77,7 @@ public class UnresolvedFunctionException extends ParseException {
* @param fct The unresolved function.
*/
public UnresolvedFunctionException(final String message, final ADQLFunction fct){
- super(message); // TODO Add the position of the function in the ADQL query!
+ super(message, (fct == null) ? null : fct.getPosition());
functionInError = fct;
}
diff --git a/src/adql/db/exception/UnresolvedJoinException.java b/src/adql/db/exception/UnresolvedJoinException.java
index 162bcd0..c73e464 100644
--- a/src/adql/db/exception/UnresolvedJoinException.java
+++ b/src/adql/db/exception/UnresolvedJoinException.java
@@ -26,8 +26,8 @@ import adql.query.TextPosition;
* This exception is thrown when a table between 2 tables can not be resolved,
* and particularly because of the join condition (i.e. column names not found, ...).
*
- * @author Grégory Mantelet (ARI) - gmantele@ari.uni-heidelberg.de
- * @version 1.3 (05/2015)
+ * @author Grégory Mantelet (ARI)
+ * @version 1.4 (06/2015)
* @since 1.2
*/
public class UnresolvedJoinException extends ParseException {
@@ -53,4 +53,15 @@ public class UnresolvedJoinException extends ParseException {
super(message, errorPosition);
}
+ /**
+ * Set the position of the invalid JOIN.
+ *
+ * @param pos Position of the concerned JOIN inside the ADQL query.
+ *
+ * @since 1.4
+ */
+ public void setPosition(final TextPosition pos){
+ this.position = pos;
+ }
+
}
diff --git a/src/adql/query/from/ADQLJoin.java b/src/adql/query/from/ADQLJoin.java
index 31b4f74..7973b2b 100644
--- a/src/adql/query/from/ADQLJoin.java
+++ b/src/adql/query/from/ADQLJoin.java
@@ -364,64 +364,69 @@ public abstract class ADQLJoin implements ADQLObject, FromContent {
@Override
public SearchColumnList getDBColumns() throws UnresolvedJoinException{
- SearchColumnList list = new SearchColumnList();
- SearchColumnList leftList = leftTable.getDBColumns();
- SearchColumnList rightList = rightTable.getDBColumns();
-
- /* 1. Figure out duplicated columns */
- HashMap<String,DBCommonColumn> mapDuplicated = new HashMap<String,DBCommonColumn>();
- // CASE: NATURAL
- if (natural){
- // Find duplicated items between the two lists and add one common column in mapDuplicated for each
- DBColumn rightCol;
- for(DBColumn leftCol : leftList){
- // search for at most one column with the same name in the RIGHT list
- // and throw an exception is there are several matches:
- rightCol = findAtMostOneColumn(leftCol.getADQLName(), (byte)0, rightList, false);
- // if there is one...
- if (rightCol != null){
- // ...check there is only one column with this name in the LEFT list,
- // and throw an exception if it is not the case:
- findExactlyOneColumn(leftCol.getADQLName(), (byte)0, leftList, true);
- // ...create a common column:
- mapDuplicated.put(leftCol.getADQLName().toLowerCase(), new DBCommonColumn(leftCol, rightCol));
+ try{
+ SearchColumnList list = new SearchColumnList();
+ SearchColumnList leftList = leftTable.getDBColumns();
+ SearchColumnList rightList = rightTable.getDBColumns();
+
+ /* 1. Figure out duplicated columns */
+ HashMap<String,DBCommonColumn> mapDuplicated = new HashMap<String,DBCommonColumn>();
+ // CASE: NATURAL
+ if (natural){
+ // Find duplicated items between the two lists and add one common column in mapDuplicated for each
+ DBColumn rightCol;
+ for(DBColumn leftCol : leftList){
+ // search for at most one column with the same name in the RIGHT list
+ // and throw an exception is there are several matches:
+ rightCol = findAtMostOneColumn(leftCol.getADQLName(), (byte)0, rightList, false);
+ // if there is one...
+ if (rightCol != null){
+ // ...check there is only one column with this name in the LEFT list,
+ // and throw an exception if it is not the case:
+ findExactlyOneColumn(leftCol.getADQLName(), (byte)0, leftList, true);
+ // ...create a common column:
+ mapDuplicated.put(leftCol.getADQLName().toLowerCase(), new DBCommonColumn(leftCol, rightCol));
+ }
}
- }
- }
- // CASE: USING
- else if (lstColumns != null && !lstColumns.isEmpty()){
- // For each columns of usingList, check there is in each list exactly one matching column, and then, add it in mapDuplicated
- DBColumn leftCol, rightCol;
- for(ADQLColumn usingCol : lstColumns){
- // search for exactly one column with the same name in the LEFT list
- // and throw an exception if there is none, or if there are several matches:
- leftCol = findExactlyOneColumn(usingCol.getColumnName(), usingCol.getCaseSensitive(), leftList, true);
- // idem in the RIGHT list:
- rightCol = findExactlyOneColumn(usingCol.getColumnName(), usingCol.getCaseSensitive(), rightList, false);
- // create a common column:
- mapDuplicated.put((usingCol.isCaseSensitive(IdentifierField.COLUMN) ? ("\"" + usingCol.getColumnName() + "\"") : usingCol.getColumnName().toLowerCase()), new DBCommonColumn(leftCol, rightCol));
}
+ // CASE: USING
+ else if (lstColumns != null && !lstColumns.isEmpty()){
+ // For each columns of usingList, check there is in each list exactly one matching column, and then, add it in mapDuplicated
+ DBColumn leftCol, rightCol;
+ for(ADQLColumn usingCol : lstColumns){
+ // search for exactly one column with the same name in the LEFT list
+ // and throw an exception if there is none, or if there are several matches:
+ leftCol = findExactlyOneColumn(usingCol.getColumnName(), usingCol.getCaseSensitive(), leftList, true);
+ // idem in the RIGHT list:
+ rightCol = findExactlyOneColumn(usingCol.getColumnName(), usingCol.getCaseSensitive(), rightList, false);
+ // create a common column:
+ mapDuplicated.put((usingCol.isCaseSensitive(IdentifierField.COLUMN) ? ("\"" + usingCol.getColumnName() + "\"") : usingCol.getColumnName().toLowerCase()), new DBCommonColumn(leftCol, rightCol));
+ }
- }
- // CASE: NO DUPLICATION TO FIGURE OUT
- else{
- // Return the union of both lists:
- list.addAll(leftList);
- list.addAll(rightList);
- return list;
- }
+ }
+ // CASE: NO DUPLICATION TO FIGURE OUT
+ else{
+ // Return the union of both lists:
+ list.addAll(leftList);
+ list.addAll(rightList);
+ return list;
+ }
- /* 2. Add all columns of the left list except the ones identified as duplications */
- addAllExcept(leftList, list, mapDuplicated);
+ /* 2. Add all columns of the left list except the ones identified as duplications */
+ addAllExcept(leftList, list, mapDuplicated);
- /* 3. Add all columns of the right list except the ones identified as duplications */
- addAllExcept(rightList, list, mapDuplicated);
+ /* 3. Add all columns of the right list except the ones identified as duplications */
+ addAllExcept(rightList, list, mapDuplicated);
- /* 4. Add all common columns of mapDuplicated */
- list.addAll(mapDuplicated.values());
+ /* 4. Add all common columns of mapDuplicated */
+ list.addAll(mapDuplicated.values());
- return list;
+ return list;
+ }catch(UnresolvedJoinException uje){
+ uje.setPosition(position);
+ throw uje;
+ }
}
public final static void addAllExcept(final SearchColumnList itemsToAdd, final SearchColumnList target, final Map<String,DBCommonColumn> exception){
diff --git a/test/adql/db/TestDBChecker.java b/test/adql/db/TestDBChecker.java
index 3f9924a..36b85b7 100644
--- a/test/adql/db/TestDBChecker.java
+++ b/test/adql/db/TestDBChecker.java
@@ -315,7 +315,7 @@ public class TestDBChecker {
assertTrue(pe instanceof UnresolvedIdentifiersException);
UnresolvedIdentifiersException ex = (UnresolvedIdentifiersException)pe;
assertEquals(1, ex.getNbErrors());
- assertEquals("Coordinate system \"fk5 geocenter\" (= \"FK5 GEOCENTER SPHERICAL2\") not allowed in this implementation.", ex.getErrors().next().getMessage());
+ assertEquals("Coordinate system \"fk5 geocenter\" (= \"FK5 GEOCENTER SPHERICAL2\") not allowed in this implementation. Allowed coordinate systems are: fk4 geocenter *, galactic * spherical2, icrs * *", ex.getErrors().next().getMessage());
}
try{
parser.parseQuery("SELECT Region('not(position fk5 heliocenter 1 2)') FROM foo;");
@@ -324,7 +324,7 @@ public class TestDBChecker {
assertTrue(pe instanceof UnresolvedIdentifiersException);
UnresolvedIdentifiersException ex = (UnresolvedIdentifiersException)pe;
assertEquals(1, ex.getNbErrors());
- assertEquals("Coordinate system \"FK5 HELIOCENTER\" (= \"FK5 HELIOCENTER SPHERICAL2\") not allowed in this implementation.", ex.getErrors().next().getMessage());
+ assertEquals("Coordinate system \"FK5 HELIOCENTER\" (= \"FK5 HELIOCENTER SPHERICAL2\") not allowed in this implementation. Allowed coordinate systems are: fk4 geocenter *, galactic * spherical2, icrs * *", ex.getErrors().next().getMessage());
}
// Test with a coordinate system while none is allowed:
@@ -345,8 +345,8 @@ public class TestDBChecker {
UnresolvedIdentifiersException ex = (UnresolvedIdentifiersException)pe;
assertEquals(2, ex.getNbErrors());
Iterator<ParseException> itErrors = ex.getErrors();
- assertEquals("Coordinate system \"ICRS SPHERICAL2\" (= \"ICRS UNKNOWNREFPOS SPHERICAL2\") not allowed in this implementation.", itErrors.next().getMessage());
- assertEquals("Coordinate system \"icrs\" (= \"ICRS UNKNOWNREFPOS SPHERICAL2\") not allowed in this implementation.", itErrors.next().getMessage());
+ assertEquals("Coordinate system \"ICRS SPHERICAL2\" (= \"ICRS UNKNOWNREFPOS SPHERICAL2\") not allowed in this implementation. No coordinate system is allowed!", itErrors.next().getMessage());
+ assertEquals("Coordinate system \"icrs\" (= \"ICRS UNKNOWNREFPOS SPHERICAL2\") not allowed in this implementation. No coordinate system is allowed!", itErrors.next().getMessage());
}
try{
parser.parseQuery("SELECT Region('not(position fk4 1 2)') FROM foo;");
@@ -355,7 +355,7 @@ public class TestDBChecker {
assertTrue(pe instanceof UnresolvedIdentifiersException);
UnresolvedIdentifiersException ex = (UnresolvedIdentifiersException)pe;
assertEquals(1, ex.getNbErrors());
- assertEquals("Coordinate system \"FK4\" (= \"FK4 UNKNOWNREFPOS SPHERICAL2\") not allowed in this implementation.", ex.getErrors().next().getMessage());
+ assertEquals("Coordinate system \"FK4\" (= \"FK4 UNKNOWNREFPOS SPHERICAL2\") not allowed in this implementation. No coordinate system is allowed!", ex.getErrors().next().getMessage());
}
}
--
GitLab