Commit db0dfdad authored by gmantele's avatar gmantele
Browse files

[ADQL] Append an HINT message in the ParseException message when an ADQL 

reserved word is encountered instead of a column/table/schema name/alias.

No list of ADQL reserved words has been added into the ADQL grammar.

However, the ADQL grammar has been slightly changed in order to provide a more
precise location of the REAL wrong part of the query.

Before this commit, if an ADQL reserved word (e.g. 'point') was encountered
outside of its normal syntax (e.g. 'point' no followed by an opening
parenthesis), the next token was highlighted instead of this one. Hence a
confusing error message.

For instance, the following ADQL query:

```sql
SELECT point
FROM aTable
```

returned the following error message:

> Encountered "FROM". Was expecting: "("

Now, it will return the following one:

> Encountered "point". Was expecting one of: "*" <QUANTIFIER> "TOP" [...]
> (HINT: "point" is a reserved ADQL word. To use it as a column/table/schema name/alias, write it between double quotes.)

This error message highlights exactly the source of the problem and even provide
to the user a clear explanation of why the query did not parse and how it could
be solved.
parent 993ee846

src/adql/parser/ADQLParser.java

+668 −680

File changed.

Preview size limit exceeded, changes collapsed.

src/adql/parser/ParseException.java

+37 −5
Original line number Diff line number Diff line
@@ -3,7 +3,17 @@ 
 * 

 * Modified by Gr&eacute;gory Mantelet (CDS), on March 2017

 * Modifications:

 *     - several small modifications.

 *     - addition of a getPosition() function in order to get the exact location

 *       of the error in the original ADQL query

 *     - generate the error message at creation instead of doing that at each

 *       call of getMessage()

 *     - use of StringBuffer to build the error message

 *     - small other alterations of the generated error message

 * 

 * Modified by Gr&eacute;gory Mantelet (ARI), on Sept. 2017

 * Modifications:

 *     - addition of a HINT in the error message when an ADQL reserved

 *       word is at the origin of the error (see initialise(...))

 * 

 * /!\ DO NOT RE-GENERATE THIS FILE /!\

 * In case of re-generation, replace it by ParseException.java.backup (but maybe
@@ -95,6 +105,22 @@ public class ParseException extends Exception { 
	/** Line in the ADQL query where the exception occurs. */

	protected TextPosition position = null;



	/** Regular expression listing all ADQL reserved words.

	 * 

	 * <p><i>Note 1:

	 * 	This list is built NOT from the list given in the ADQL-2.0 standard,

	 * 	but from the collation of all words potentially used in an ADQL query

	 * 	(including standard function names).

	 * </i></p>

	 * 

	 * <p><i>Note 2:

	 * 	This regular expression is only used to display an appropriate hint

	 * 	to the user in the error message if a such word is at the origin of

	 * 	the error. (see {@link #initialise(Token, int[][], String[])} for more

	 * 	details).

	 * </i></p> */

	private final static String ADQL_RESERVED_WORDS_REGEX = "(ABS|ACOS|AREA|ASIN|ATAN|ATAN2|BOX|CEILING|CENTROID|CIRCLE|CONTAINS|COORD1|COORD2|COORDSYS|COS|DEGREES|DISTANCE|EXP|FLOOR|INTERSECTS|LOG|LOG10|MOD|PI|POINT|POLYGON|POWER|RADIANS|REGION|RAND|ROUND|SIN|SQRT|TOP|TAN|TRUNCATE|SELECT|TOP|DISTINCT|ALL|AS|COUNT|AVG|MAX|MIN|SUM|FROM|JOIN|CROSS|INNER|OUTER|LEFT|RIGHT|FULL|NATURAL|USING|ON|WHERE|IS|NOT|AND|OR|EXISTS|IN|LIKE|NULL|BETWEEN|ORDER|ASC|DESC|GROUP|BY|HAVING)";



	/**

	 * Gets the position in the ADQL query of the token which generates this exception.

	 *
@@ -130,17 +156,18 @@ public class ParseException extends Exception { 
		StringBuffer msg = new StringBuffer();

		msg.append(" Encountered \"");

		Token tok = currentToken.next;

		StringBuffer tokenName = new StringBuffer();

		for(int i = 0; i < maxSize; i++){

			if (i != 0)

				msg.append(' ');

				tokenName.append(' ');

			if (tok.kind == 0){

				msg.append(tokenImage[0]);

				tokenName.append(tokenImage[0]);

				break;

			}

			msg.append(add_escapes(tok.image));

			tokenName.append(add_escapes(tok.image));

			tok = tok.next;

		}

		msg.append("\".");

		msg.append(tokenName.toString()).append("\".");



		// Append the expected tokens list:

		if (expectedTokenSequences.length == 1){
@@ -150,6 +177,11 @@ public class ParseException extends Exception { 
		}

		msg.append(expected);



		// Append a hint about reserved words if it is one:

		String word = tokenName.toString().trim();

		if (word.toUpperCase().matches(ADQL_RESERVED_WORDS_REGEX))

			msg.append(System.getProperty("line.separator", "\n")).append("(HINT: \"").append(word).append("\" is a reserved ADQL word. To use it as a column/table/schema name/alias, write it between double quotes.)");



		return msg.toString();

		/*String eol = System.getProperty("line.separator", "\n");

		StringBuffer expected = new StringBuffer();

src/adql/parser/ParseException.java.backup

+37 −5
Original line number Diff line number Diff line
@@ -3,7 +3,17 @@ 
 * 

 * Modified by Gr&eacute;gory Mantelet (CDS), on March 2017

 * Modifications:

 *     - several small modifications.

 *     - addition of a getPosition() function in order to get the exact location

 *       of the error in the original ADQL query

 *     - generate the error message at creation instead of doing that at each

 *       call of getMessage()

 *     - use of StringBuffer to build the error message

 *     - small other alterations of the generated error message

 * 

 * Modified by Gr&eacute;gory Mantelet (ARI), on Sept. 2017

 * Modifications:

 *     - addition of a HINT in the error message when an ADQL reserved

 *       word is at the origin of the error (see initialise(...))

 * 

 * /!\ DO NOT RE-GENERATE THIS FILE /!\

 * In case of re-generation, replace it by ParseException.java.backup (but maybe
@@ -95,6 +105,22 @@ public class ParseException extends Exception { 
	/** Line in the ADQL query where the exception occurs. */

	protected TextPosition position = null;



	/** Regular expression listing all ADQL reserved words.

	 * 

	 * <p><i>Note 1:

	 * 	This list is built NOT from the list given in the ADQL-2.0 standard,

	 * 	but from the collation of all words potentially used in an ADQL query

	 * 	(including standard function names).

	 * </i></p>

	 * 

	 * <p><i>Note 2:

	 * 	This regular expression is only used to display an appropriate hint

	 * 	to the user in the error message if a such word is at the origin of

	 * 	the error. (see {@link #initialise(Token, int[][], String[])} for more

	 * 	details).

	 * </i></p> */

	private final static String ADQL_RESERVED_WORDS_REGEX = "(ABS|ACOS|AREA|ASIN|ATAN|ATAN2|BOX|CEILING|CENTROID|CIRCLE|CONTAINS|COORD1|COORD2|COORDSYS|COS|DEGREES|DISTANCE|EXP|FLOOR|INTERSECTS|LOG|LOG10|MOD|PI|POINT|POLYGON|POWER|RADIANS|REGION|RAND|ROUND|SIN|SQRT|TOP|TAN|TRUNCATE|SELECT|TOP|DISTINCT|ALL|AS|COUNT|AVG|MAX|MIN|SUM|FROM|JOIN|CROSS|INNER|OUTER|LEFT|RIGHT|FULL|NATURAL|USING|ON|WHERE|IS|NOT|AND|OR|EXISTS|IN|LIKE|NULL|BETWEEN|ORDER|ASC|DESC|GROUP|BY|HAVING)";



	/**

	 * Gets the position in the ADQL query of the token which generates this exception.

	 *
@@ -130,17 +156,18 @@ public class ParseException extends Exception { 
		StringBuffer msg = new StringBuffer();

		msg.append(" Encountered \"");

		Token tok = currentToken.next;

		StringBuffer tokenName = new StringBuffer();

		for(int i = 0; i < maxSize; i++){

			if (i != 0)

				msg.append(' ');

				tokenName.append(' ');

			if (tok.kind == 0){

				msg.append(tokenImage[0]);

				tokenName.append(tokenImage[0]);

				break;

			}

			msg.append(add_escapes(tok.image));

			tokenName.append(add_escapes(tok.image));

			tok = tok.next;

		}

		msg.append("\".");

		msg.append(tokenName.toString()).append("\".");



		// Append the expected tokens list:

		if (expectedTokenSequences.length == 1){
@@ -150,6 +177,11 @@ public class ParseException extends Exception { 
		}

		msg.append(expected);



		// Append a hint about reserved words if it is one:

		String word = tokenName.toString().trim();

		if (word.toUpperCase().matches(ADQL_RESERVED_WORDS_REGEX))

			msg.append(System.getProperty("line.separator", "\n")).append("(HINT: \"").append(word).append("\" is a reserved ADQL word. To use it as a column/table/schema name/alias, write it between double quotes.)");



		return msg.toString();

		/*String eol = System.getProperty("line.separator", "\n");

		StringBuffer expected = new StringBuffer();

src/adql/parser/adqlGrammar.jj

+13 −2
Original line number Diff line number Diff line
@@ -1192,10 +1192,21 @@ ADQLOperand ValueExpression(): {ADQLOperand valueExpr = null; Token left, right; 
		| LOOKAHEAD(<COORDSYS> | (StringFactor() <CONCAT>)) valueExpr=StringExpression()

		| LOOKAHEAD(<LEFT_PAR>) left=<LEFT_PAR> valueExpr=ValueExpression() right=<RIGHT_PAR> { valueExpr = queryFactory.createWrappedOperand(valueExpr); ((WrappedOperand)valueExpr).setPosition(new TextPosition(left, right)); }

		| LOOKAHEAD(<REGULAR_IDENTIFIER> <LEFT_PAR>) valueExpr=UserDefinedFunction()

		| valueExpr=GeometryValueFunction()

		| LOOKAHEAD(2) valueExpr=GeometryValueFunction()

		| LOOKAHEAD(Column()) valueExpr=Column()

		| LOOKAHEAD(String()) valueExpr=StringFactor()

		| valueExpr=Factor())

		| LOOKAHEAD(3) valueExpr=Factor()



		/* At this position in this switch, all possibilities (including

		 * Column()) have already been tested and failed.

		 * 

		 * So, this final choice actually aims to throw an error set with the

		 * current token and with an error message implying that a column name

		 * was expected (which is generally the case in an ADQL query).

		 *

		 * Note: This choice will generally be reached if an unexpected ADQL

		 *       word is ending the query. */

		| valueExpr=Column() )

		{return valueExpr;}

	}catch(Exception ex){

		throw generateParseException(ex);

test/adql/parser/TestADQLParser.java

+78 −0
Original line number Diff line number Diff line
@@ -206,4 +206,82 @@ public class TestADQLParser { 
		}

	}



	@Test

	public void testADQLReservedWord(){

		ADQLParser parser = new ADQLParser();



		final String hintAbs = "\n(HINT: \"abs\" is a reserved ADQL word. To use it as a column/table/schema name/alias, write it between double quotes.)";

		final String hintPoint = "\n(HINT: \"point\" is a reserved ADQL word. To use it as a column/table/schema name/alias, write it between double quotes.)";

		final String hintExists = "\n(HINT: \"exists\" is a reserved ADQL word. To use it as a column/table/schema name/alias, write it between double quotes.)";

		final String hintLike = "\n(HINT: \"LIKE\" is a reserved ADQL word. To use it as a column/table/schema name/alias, write it between double quotes.)";



		/* TEST AS A COLUMN/TABLE/SCHEMA NAME... */

		// ...with a numeric function name (but no param):

		try{

			parser.parseQuery("select abs from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintAbs));

		}

		// ...with a geometric function name (but no param):

		try{

			parser.parseQuery("select point from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintPoint));

		}

		// ...with an ADQL function name (but no param):

		try{

			parser.parseQuery("select exists from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintExists));

		}

		// ...with an ADQL syntax item:

		try{

			parser.parseQuery("select LIKE from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintLike));

		}



		/* TEST AS AN ALIAS... */

		// ...with a numeric function name (but no param):

		try{

			parser.parseQuery("select aCol AS abs from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintAbs));

		}

		// ...with a geometric function name (but no param):

		try{

			parser.parseQuery("select aCol AS point from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintPoint));

		}

		// ...with an ADQL function name (but no param):

		try{

			parser.parseQuery("select aCol AS exists from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintExists));

		}

		// ...with an ADQL syntax item:

		try{

			parser.parseQuery("select aCol AS LIKE from aTable");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintLike));

		}



		/* TEST AT THE END OF THE QUERY (AND IN A WHERE) */

		try{

			parser.parseQuery("select aCol from aTable WHERE toto = abs");

		}catch(Throwable t){

			assertEquals(ParseException.class, t.getClass());

			assertTrue(t.getMessage().endsWith(hintAbs));

		}

	}



}